* [PATCH v2 net-next] net: sk_buff rbnode reorg
From: Eric Dumazet @ 2017-09-19 12:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Soheil Hassas Yeganeh, Wei Wang, Willem de Bruijn
In-Reply-To: <1505797616.29839.40.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
skb->rbnode shares space with skb->next, skb->prev and skb->tstamp
Current uses (TCP receive ofo queue and netem) need to save/restore
tstamp, while skb->dev is either NULL (TCP) or a constant for a given
queue (netem).
Since we plan using an RB tree for TCP retransmit queue to speedup SACK
processing with large BDP, this patch exchanges skb->dev and
skb->tstamp.
This saves some overhead in both TCP and netem.
v2: removes the swtstamp field from struct tcp_skb_cb
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Wei Wang <weiwan@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
include/linux/skbuff.h | 16 ++++++++--------
include/net/tcp.h | 6 ------
net/ipv4/tcp_input.c | 27 +++++----------------------
net/sched/sch_netem.c | 7 ++++---
4 files changed, 17 insertions(+), 39 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 72299ef00061db1ce70d34b96ae1639ecde08837..492828801acba42ac6bccb287d3cc5080039135c 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -661,8 +661,12 @@ struct sk_buff {
struct sk_buff *prev;
union {
- ktime_t tstamp;
- u64 skb_mstamp;
+ struct net_device *dev;
+ /* Some protocols might use this space to store information,
+ * while device pointer would be NULL.
+ * UDP receive path is one user.
+ */
+ unsigned long dev_scratch;
};
};
struct rb_node rbnode; /* used in netem & tcp stack */
@@ -670,12 +674,8 @@ struct sk_buff {
struct sock *sk;
union {
- struct net_device *dev;
- /* Some protocols might use this space to store information,
- * while device pointer would be NULL.
- * UDP receive path is one user.
- */
- unsigned long dev_scratch;
+ ktime_t tstamp;
+ u64 skb_mstamp;
};
/*
* This is the control buffer. It is free to use for every
diff --git a/include/net/tcp.h b/include/net/tcp.h
index b510f284427aabc1f508d24d29d0f812e5e0aa61..49a8a46466f32bd98b0c38a776fddc40bc621337 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -797,12 +797,6 @@ struct tcp_skb_cb {
u16 tcp_gso_segs;
u16 tcp_gso_size;
};
-
- /* Used to stash the receive timestamp while this skb is in the
- * out of order queue, as skb->tstamp is overwritten by the
- * rbnode.
- */
- ktime_t swtstamp;
};
__u8 tcp_flags; /* TCP header flags. (tcp[13]) */
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bddf724f5c02abe1d11110cffe2517f6376e440d..db9bb46b5776f9ee332298c0e95afb0a5966b938 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4266,11 +4266,6 @@ static void tcp_sack_remove(struct tcp_sock *tp)
tp->rx_opt.num_sacks = num_sacks;
}
-enum tcp_queue {
- OOO_QUEUE,
- RCV_QUEUE,
-};
-
/**
* tcp_try_coalesce - try to merge skb to prior one
* @sk: socket
@@ -4286,7 +4281,6 @@ enum tcp_queue {
* Returns true if caller should free @from instead of queueing it
*/
static bool tcp_try_coalesce(struct sock *sk,
- enum tcp_queue dest,
struct sk_buff *to,
struct sk_buff *from,
bool *fragstolen)
@@ -4311,10 +4305,7 @@ static bool tcp_try_coalesce(struct sock *sk,
if (TCP_SKB_CB(from)->has_rxtstamp) {
TCP_SKB_CB(to)->has_rxtstamp = true;
- if (dest == OOO_QUEUE)
- TCP_SKB_CB(to)->swtstamp = TCP_SKB_CB(from)->swtstamp;
- else
- to->tstamp = from->tstamp;
+ to->tstamp = from->tstamp;
}
return true;
@@ -4351,9 +4342,6 @@ static void tcp_ofo_queue(struct sock *sk)
}
p = rb_next(p);
rb_erase(&skb->rbnode, &tp->out_of_order_queue);
- /* Replace tstamp which was stomped by rbnode */
- if (TCP_SKB_CB(skb)->has_rxtstamp)
- skb->tstamp = TCP_SKB_CB(skb)->swtstamp;
if (unlikely(!after(TCP_SKB_CB(skb)->end_seq, tp->rcv_nxt))) {
SOCK_DEBUG(sk, "ofo packet was already received\n");
@@ -4365,8 +4353,7 @@ static void tcp_ofo_queue(struct sock *sk)
TCP_SKB_CB(skb)->end_seq);
tail = skb_peek_tail(&sk->sk_receive_queue);
- eaten = tail && tcp_try_coalesce(sk, RCV_QUEUE,
- tail, skb, &fragstolen);
+ eaten = tail && tcp_try_coalesce(sk, tail, skb, &fragstolen);
tcp_rcv_nxt_update(tp, TCP_SKB_CB(skb)->end_seq);
fin = TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN;
if (!eaten)
@@ -4420,10 +4407,6 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
return;
}
- /* Stash tstamp to avoid being stomped on by rbnode */
- if (TCP_SKB_CB(skb)->has_rxtstamp)
- TCP_SKB_CB(skb)->swtstamp = skb->tstamp;
-
/* Disable header prediction. */
tp->pred_flags = 0;
inet_csk_schedule_ack(sk);
@@ -4451,7 +4434,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
/* In the typical case, we are adding an skb to the end of the list.
* Use of ooo_last_skb avoids the O(Log(N)) rbtree lookup.
*/
- if (tcp_try_coalesce(sk, OOO_QUEUE, tp->ooo_last_skb,
+ if (tcp_try_coalesce(sk, tp->ooo_last_skb,
skb, &fragstolen)) {
coalesce_done:
tcp_grow_window(sk, skb);
@@ -4502,7 +4485,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
__kfree_skb(skb1);
goto merge_right;
}
- } else if (tcp_try_coalesce(sk, OOO_QUEUE, skb1,
+ } else if (tcp_try_coalesce(sk, skb1,
skb, &fragstolen)) {
goto coalesce_done;
}
@@ -4554,7 +4537,7 @@ static int __must_check tcp_queue_rcv(struct sock *sk, struct sk_buff *skb, int
__skb_pull(skb, hdrlen);
eaten = (tail &&
- tcp_try_coalesce(sk, RCV_QUEUE, tail,
+ tcp_try_coalesce(sk, tail,
skb, fragstolen)) ? 1 : 0;
tcp_rcv_nxt_update(tcp_sk(sk), TCP_SKB_CB(skb)->end_seq);
if (!eaten) {
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index b1266e75ca43cf5a66b951ecabccfc5b24069444..063a4bdb9ee6f26b01387959e8f6ccd15ec16191 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -146,7 +146,6 @@ struct netem_sched_data {
*/
struct netem_skb_cb {
psched_time_t time_to_send;
- ktime_t tstamp_save;
};
@@ -561,7 +560,6 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
}
cb->time_to_send = now + delay;
- cb->tstamp_save = skb->tstamp;
++q->counter;
tfifo_enqueue(skb, sch);
} else {
@@ -629,7 +627,10 @@ static struct sk_buff *netem_dequeue(struct Qdisc *sch)
qdisc_qstats_backlog_dec(sch, skb);
skb->next = NULL;
skb->prev = NULL;
- skb->tstamp = netem_skb_cb(skb)->tstamp_save;
+ /* skb->dev shares skb->rbnode area,
+ * we need to restore its value.
+ */
+ skb->dev = qdisc_dev(sch);
#ifdef CONFIG_NET_CLS_ACT
/*
^ permalink raw reply related
* Re: [PATCH net-next 07/14] gtp: Support encapsulation of IPv6 packets
From: Harald Welte @ 2017-09-19 12:12 UTC (permalink / raw)
To: David Miller; +Cc: tom, netdev, pablo, rohit
In-Reply-To: <20170918.211908.2152170523885516973.davem@davemloft.net>
Hi Dave,
On Mon, Sep 18, 2017 at 09:19:08PM -0700, David Miller wrote:
> > +static inline u32 ipv6_hashfn(const struct in6_addr *a)
> > +{
> > + return __ipv6_addr_jhash(a, gtp_h_initval);
> > +}
>
> I know you are just following the pattern of the existing "ipv4_hashfn()" here
> but this kind of stuff is not very global namespace friendly. Even simply
> adding a "gtp_" prefix to these hash functions would be a lot better.
I would agree if this was an inline function defined in a header file or
a non-static function. But where is the global namespace concern in
case of static inline functions defined and used in the same .c file?
If it makes you happy, I'm all for adding the prefix - I just would like
to understand the rationale better, thanks :)
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 08/14] gtp: Support encpasulating over IPv6
From: Harald Welte @ 2017-09-19 11:59 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, pablo, rohit
In-Reply-To: <20170919003904.5124-9-tom@quantonium.net>
Hi Tom,
On Mon, Sep 18, 2017 at 05:38:58PM -0700, Tom Herbert wrote:
> Allow peers to be specified by IPv6 addresses.
> + u16 peer_af;
> + union {
> + struct in_addr peer_addr_ip4;
> + struct in6_addr peer_addr_ip6;
> + };
this will not really work, as an union means that a PDP context
will be either IPv4-only or IPV6-only, while in reality there
are three types, see my other mail. So you have to deal
with v4-only, v6-only or v4v6.
The v6-only is legacy by now, and all modern phones I've tested in
recent years can do v4v6 rather than having a v4-only and a v6-only
PDP context in parallel.
>From the operator point of view, v4v6 is very desirable, as it basically
halves the amount of PDP contexts compared to the old approach, which
significantly reduces signalling load across your network, as well as
the amount of memory (and thus capacity) in your core network elements.
I've recently implemented v6 + v4v6 support in osmo-ggsn (see
http://git.osmocom.org/osmo-ggsn/) in case you would like to see another
FOSS implementation for v6 + v4v6 - though in userspace, of course.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 13/14] gtp: Support for GRO
From: Harald Welte @ 2017-09-19 12:03 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, pablo, rohit
In-Reply-To: <20170919003904.5124-14-tom@quantonium.net>
On Mon, Sep 18, 2017 at 05:39:03PM -0700, Tom Herbert wrote:
> Populate GRO receive and GRO complete functions for GTP-Uv0 and v1.
looks fine to me, though I'm not the GRO expert here. Let's say what
the netdev gurus have to say in their review.
If you say it is tested with GRO-capable and non-GRO capable device
drivers, I'm fine with the patch.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 04/14] gtp: udp recv clean up
From: Harald Welte @ 2017-09-19 11:32 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, pablo, rohit
In-Reply-To: <20170919003904.5124-5-tom@quantonium.net>
Hi Tom,
I think this patch does too many things at once:
* introduce separate rx functions
* convert from netif_rx to gro_cells_receive
* cosmetic changes like "return -1" to "goto drop"
In the context of reviewability and the "one patch per topic", I would
prefer to see those separated, thanks.
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 07/14] gtp: Support encapsulation of IPv6 packets
From: Harald Welte @ 2017-09-19 11:53 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, pablo, rohit
In-Reply-To: <20170919003904.5124-8-tom@quantonium.net>
On Mon, Sep 18, 2017 at 05:38:57PM -0700, Tom Herbert wrote:
> Allow IPv6 mobile subscriber packets. This entails adding an IPv6 mobile
> subscriber address to pdp context and IPv6 specific variants to find pdp
> contexts by address.
Please note that there are three different PDP contexts for IP:
* IPv4 only (what gtp.c implements so far)
* IPv6 only
* dual IPv4+IPv6 (called IPv46)
This information will have to be provisioned by the control plane
via netlink for each PDP context. The kernel module then needs to
make sure that on a v4-only context no IPv6 packets are accepted
and vice-versa.
Your proposed patch is missing this kind of screening function and
I would imagine it could introduce all kinds of security problems :/
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 05/14] gtp: Remove special mtu handling
From: Harald Welte @ 2017-09-19 11:42 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, pablo, rohit
In-Reply-To: <20170919003904.5124-6-tom@quantonium.net>
Hi Tom,
On Mon, Sep 18, 2017 at 05:38:55PM -0700, Tom Herbert wrote:
> Removes MTU handling in gtp_build_skb_ip4. This is non standard relative
> to how other tunneling protocols handle MTU. The model espoused is that
> the inner interface should set it's MTU to be less than the expected
> path MTU on the overlay network. Path MTU discovery is not typically
> used for modifying tunnel MTUs.
The point of the kernel GTP module is to interoperate with existing
other GTP implementations and the practises established by cellular
operators when operating GTP in their networks.
While what you describe (chose interface MTU to be less than the
expected path MTU) is generally best practise in the Linux IP/networking
world, this is not generally reflected in the cellular
universe. You see quite a bit of GTP fragmentation due to the fact
that the transport network simply has to deal with the MTU that has
been established via the control plane between SGSN and MS/UE, without
the GGSN even being part of that negotiation.
Also, you may very well have one "gtp0" tunnel device at the GGSN,
but you are establishing individual GTP tunnels to dozesn to hundreds of
different SGSNs at operators all over the world. You cannot reliably
set the "gtp0" interface MTU to "the path MTU of the overlay network",
as the overlay network is in fact different for each of the SGSNs you're
talking to - and each may have a different path MTU.
So unless I'm missing something, I would currently vote for staying with
the current code, which uses the path MTU to the specific destination IP
address (the SGSN).
Regards,
Harald
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next 03/14] gtp: Call common functions to get tunnel routes and add dst_cache
From: Harald Welte @ 2017-09-19 12:09 UTC (permalink / raw)
To: David Miller; +Cc: tom, netdev, pablo, rohit
In-Reply-To: <20170918.211751.1871429944584121281.davem@davemloft.net>
Hi Dave,
On Mon, Sep 18, 2017 at 09:17:51PM -0700, David Miller wrote:
> This and the new dst caching code ignores any source address selection
> done by ip_route_output_key() or the new tunnel route lookup helpers.
>
> Either source address selection should be respected, or if saddr will
> never be modified by a route lookup for some specific reason here,
> that should be documented.
The IP source address is fixed by signaling on the GTP-C control plane
and nothing that the kernel can unilaterally decide to change. Such a
change of address would have to be decided by and first be signaled on
GTP-C to the peer by the userspace daemon, which would then update the
PDP context in the kernel.
So I guess you're asking us to document that rationale as form of a
source code comment ?
--
- Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
^ permalink raw reply
* Re: [PATCH net-next] net: sk_buff rbnode reorg
From: Eric Dumazet @ 2017-09-19 12:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <1505797616.29839.40.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 2017-09-18 at 22:06 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
...
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -797,11 +797,6 @@ struct tcp_skb_cb {
> u16 tcp_gso_segs;
> u16 tcp_gso_size;
> };
> -
> - /* Used to stash the receive timestamp while this skb is in the
> - * out of order queue, as skb->tstamp is overwritten by the
> - * rbnode.
> - */
> ktime_t swtstamp;
> };
I forgot that in upstream kernels, swtstamp can also be removed.
It is used in our kernels to store the time at which the skb was moved
into socket receive queue (after a possible stay in out of order queue)
Will send a V2, removing this field from net-next, before we add it
again later if we upstream our instrumentation code.
^ permalink raw reply
* Re: [PATCH net-next] net: avoid a full fib lookup when rp_filter is disabled.
From: Eric Dumazet @ 2017-09-19 12:00 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, Hannes Frederic Sowa
In-Reply-To: <1967ebaa528c626ebd9682e927fd33770a396a0a.1505817863.git.pabeni@redhat.com>
On Tue, 2017-09-19 at 12:46 +0200, Paolo Abeni wrote:
..
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> +/* called under RCU lock */
> +struct in_ifaddr *inet_lookup_ifaddr_rcu(struct net *net, __be32 addr)
> +{
> + u32 hash = inet_addr_hash(net, addr);
> + struct in_ifaddr *ifa;
> +
> + hlist_for_each_entry_rcu(ifa, &inet_addr_lst[hash], hash)
> + if (ifa->ifa_local == addr &&
> + net_eq(dev_net(ifa->ifa_dev->dev), net))
> + return ifa;
> +
> + return NULL;
> +}
> +
Any particular reason you do not use this helper to replace the lookup
in __ip_dev_find() ?
In this case, copy paste can be avoided, please avoid a future patch
consolidating the things...
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 13/14] gtp: Support for GRO
From: kbuild test robot @ 2017-09-19 11:57 UTC (permalink / raw)
To: Tom Herbert; +Cc: kbuild-all, davem, netdev, pablo, laforge, rohit, Tom Herbert
In-Reply-To: <20170919003904.5124-14-tom@quantonium.net>
Hi Tom,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tom-Herbert/gtp-Additional-feature-support/20170919-143920
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
vim +3958 include/linux/netdevice.h
5b33bc6e Tom Herbert 2017-09-18 3944
5b33bc6e Tom Herbert 2017-09-18 3945 /* rcu_read_lock() must be held */
5b33bc6e Tom Herbert 2017-09-18 3946 static inline struct skb_gso_app *skb_gso_app_lookup(struct sk_buff *skb,
5b33bc6e Tom Herbert 2017-09-18 3947 netdev_features_t features,
5b33bc6e Tom Herbert 2017-09-18 3948 unsigned int check_flags)
5b33bc6e Tom Herbert 2017-09-18 3949 {
5b33bc6e Tom Herbert 2017-09-18 3950 struct skb_gso_app *app;
5b33bc6e Tom Herbert 2017-09-18 3951 int type;
5b33bc6e Tom Herbert 2017-09-18 3952
5b33bc6e Tom Herbert 2017-09-18 3953 if (!(skb_shinfo(skb)->gso_type & SKB_GSO_APP_MASK))
5b33bc6e Tom Herbert 2017-09-18 3954 return false;
5b33bc6e Tom Herbert 2017-09-18 3955
5b33bc6e Tom Herbert 2017-09-18 3956 type = skb_gso_app_to_index(skb_shinfo(skb)->gso_type);
5b33bc6e Tom Herbert 2017-09-18 3957
5b33bc6e Tom Herbert 2017-09-18 @3958 app = rcu_dereference(skb_gso_apps[type]);
5b33bc6e Tom Herbert 2017-09-18 3959 if (app && app->gso_segment &&
5b33bc6e Tom Herbert 2017-09-18 3960 (check_flags & app->check_flags))
5b33bc6e Tom Herbert 2017-09-18 3961 return app;
5b33bc6e Tom Herbert 2017-09-18 3962
5b33bc6e Tom Herbert 2017-09-18 3963 return NULL;
5b33bc6e Tom Herbert 2017-09-18 3964 }
5b33bc6e Tom Herbert 2017-09-18 3965
:::::: The code at line 3958 was first introduced by commit
:::::: 5b33bc6e4fcae1113167c651a3d3a218c7e277c6 net: Add a facility to support application defined GSO
:::::: TO: Tom Herbert <tom@quantonium.net>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCHv2 iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-19 11:48 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Phil Sutter
In-Reply-To: <20170919030520.GL5465@leo.usersys.redhat.com>
On Tue, Sep 19, 2017 at 11:05:20AM +0800, Hangbin Liu wrote:
> On Mon, Sep 18, 2017 at 09:55:05AM +0200, Michal Kubecek wrote:
> > > @@ -471,19 +516,23 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > >
> > > if (h->nlmsg_type == NLMSG_ERROR) {
> > > rtnl_dump_error(rth, h);
> > > + free(buf);
> > > return -1;
> > > }
> > >
> > > if (!rth->dump_fp) {
> > > err = a->filter(&nladdr, h, a->arg1);
> > > - if (err < 0)
> > > + if (err < 0) {
> > > + free(buf);
> > > return err;
> > > + }
> > > }
> > >
> > > skip_it:
> > > h = NLMSG_NEXT(h, msglen);
> > > }
> > > }
> > > + free(buf);
> >
> > We only free the last buffer returned by rtnl_recvmsg() this way.
> > IMHO this free(buf) should be moved inside the loop.
>
> Do you mean the outside while loop or the for loop? I think we could
> not put it inside the for loop, because we may need the buf multi
> times based on arg.
Sorry for the confusion, you are right, this part is correct. I misread
the indentation.
Michal Kubecek
^ permalink raw reply
* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Oleksandr Natalenko @ 2017-09-19 11:04 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Netdev
In-Reply-To: <CAK6E8=e7SE-M4r2nJ8K_FoDMVcN2xYdtw=MetvD5MvXMm9iA1Q@mail.gmail.com>
Hi.
18.09.2017 23:40, Yuchung Cheng wrote:
> I assume this kernel does not have the patch that Neal proposed in his
> first reply?
Correct.
> The main warning needs to be triggered by another peculiar SACK that
> kicks the sender into recovery again (after undo). Please let it run
> longer if possible to see if we can get both. But the new data does
> indicate the we can (validly) be in CA_Open with retrans_out > 0.
OK, here it is:
===
» LC_TIME=C jctl -kb | grep RIP
…
Sep 19 12:54:03 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 19 12:54:22 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 19 12:54:25 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 19 12:56:00 defiant kernel: RIP:
0010:tcp_fastretrans_alert+0x7c8/0x990
Sep 19 12:57:07 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 19 12:57:14 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
Sep 19 12:58:04 defiant kernel: RIP:
0010:tcp_undo_cwnd_reduction+0xbd/0xd0
…
===
Note timestamps — two types of warning are distant in time, so didn't
happen at once.
While still running this kernel, anything else I can check for you?
^ permalink raw reply
* [PATCH net-next] net: avoid a full fib lookup when rp_filter is disabled.
From: Paolo Abeni @ 2017-09-19 10:46 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Hannes Frederic Sowa
Since commit 1dced6a85482 ("ipv4: Restore accept_local
behaviour in fib_validate_source()") a full fib lookup
is needed even if the rp_filter is disabled, if
accept_local is false - which is the default.
What we really need in the above scenario is just checking
that the source IP address is not local, and we can do
that is a cheaper way looking up the ifaddr hash table.
This commit adds an helper for such lookup and uses it
to validate the src address when rp_filter is disabled.
It also drops the checks to bail early from
__fib_validate_source, added by the commit 1dced6a85482
("ipv4: Restore accept_local behaviour in fib_validate_source()")
and now unneeded.
This improves UDP performances for unconnected sockets
when rp_filter is disabled by 5% and also gives small but
measurable performance improvement for TCP flood scenarios.
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/linux/inetdevice.h | 1 +
net/ipv4/devinet.c | 14 ++++++++++++++
net/ipv4/fib_frontend.c | 11 +++++++----
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index fb3f809e34e4..751d051f0bc7 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -179,6 +179,7 @@ __be32 inet_confirm_addr(struct net *net, struct in_device *in_dev, __be32 dst,
__be32 local, int scope);
struct in_ifaddr *inet_ifa_byprefix(struct in_device *in_dev, __be32 prefix,
__be32 mask);
+struct in_ifaddr *inet_lookup_ifaddr_rcu(struct net *net, __be32 addr);
static __inline__ bool inet_ifa_match(__be32 addr, struct in_ifaddr *ifa)
{
return !((addr^ifa->ifa_address)&ifa->ifa_mask);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index d7adc0616599..73bf09bcfe43 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -173,6 +173,20 @@ struct net_device *__ip_dev_find(struct net *net, __be32 addr, bool devref)
}
EXPORT_SYMBOL(__ip_dev_find);
+/* called under RCU lock */
+struct in_ifaddr *inet_lookup_ifaddr_rcu(struct net *net, __be32 addr)
+{
+ u32 hash = inet_addr_hash(net, addr);
+ struct in_ifaddr *ifa;
+
+ hlist_for_each_entry_rcu(ifa, &inet_addr_lst[hash], hash)
+ if (ifa->ifa_local == addr &&
+ net_eq(dev_net(ifa->ifa_dev->dev), net))
+ return ifa;
+
+ return NULL;
+}
+
static void rtmsg_ifa(int event, struct in_ifaddr *, struct nlmsghdr *, u32);
static BLOCKING_NOTIFIER_HEAD(inetaddr_chain);
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 37819ab4cc74..1470d265a357 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -345,9 +345,6 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
if (res.type != RTN_UNICAST &&
(res.type != RTN_LOCAL || !IN_DEV_ACCEPT_LOCAL(idev)))
goto e_inval;
- if (!rpf && !fib_num_tclassid_users(net) &&
- (dev->ifindex != oif || !IN_DEV_TX_REDIRECTS(idev)))
- goto last_resort;
fib_combine_itag(itag, &res);
dev_match = false;
@@ -404,8 +401,14 @@ int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
int r = secpath_exists(skb) ? 0 : IN_DEV_RPFILTER(idev);
if (!r && !fib_num_tclassid_users(dev_net(dev)) &&
- IN_DEV_ACCEPT_LOCAL(idev) &&
(dev->ifindex != oif || !IN_DEV_TX_REDIRECTS(idev))) {
+ /* we need only to ensure that the src address is not a
+ * local one
+ */
+ if (!IN_DEV_ACCEPT_LOCAL(idev) &&
+ inet_lookup_ifaddr_rcu(dev_net(dev), src))
+ return -EINVAL;
+
*itag = 0;
return 0;
}
--
2.13.5
^ permalink raw reply related
* [PATCH] rhashtable: Documentation tweak
From: Andreas Gruenbacher @ 2017-09-19 10:41 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, Andreas Gruenbacher
Clarify that rhashtable_walk_{stop,start} will not reset the iterator to
the beginning of the hash table. Confusion between rhashtable_walk_enter
and rhashtable_walk_start has already lead to a bug.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
lib/rhashtable.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 707ca5d677c6..ddd7dde87c3c 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -735,9 +735,9 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
* rhashtable_walk_start - Start a hash table walk
* @iter: Hash table iterator
*
- * Start a hash table walk. Note that we take the RCU lock in all
- * cases including when we return an error. So you must always call
- * rhashtable_walk_stop to clean up.
+ * Start a hash table walk at the current iterator position. Note that we take
+ * the RCU lock in all cases including when we return an error. So you must
+ * always call rhashtable_walk_stop to clean up.
*
* Returns zero if successful.
*
@@ -846,7 +846,8 @@ EXPORT_SYMBOL_GPL(rhashtable_walk_next);
* rhashtable_walk_stop - Finish a hash table walk
* @iter: Hash table iterator
*
- * Finish a hash table walk.
+ * Finish a hash table walk. Does not reset the iterator to the start of the
+ * hash table.
*/
void rhashtable_walk_stop(struct rhashtable_iter *iter)
__releases(RCU)
--
2.13.3
^ permalink raw reply related
* [PATCH net-next] udp: do rmem bulk free even if the rx sk queue is empty
From: Paolo Abeni @ 2017-09-19 10:11 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Eric Dumazet
The commit 6b229cf77d68 ("udp: add batching to udp_rmem_release()")
reduced greatly the cacheline contention between the BH and the US
reader batching the rmem updates in most scenarios.
Such optimization is explicitly avoided if the US reader is faster
then BH processing.
My fault, I initially suggested this kind of behavior due to concerns
of possible regressions with small sk_rcvbuf values. Tests showed
such concerns are misplaced, so this commit relaxes the condition
for rmem bulk updates, obtaining small but measurable performance
gain in the scenario described above.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv4/udp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index ef29df8648e4..784ced0b9150 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1212,8 +1212,7 @@ static void udp_rmem_release(struct sock *sk, int size, int partial,
if (likely(partial)) {
up->forward_deficit += size;
size = up->forward_deficit;
- if (size < (sk->sk_rcvbuf >> 2) &&
- !skb_queue_empty(&up->reader_queue))
+ if (size < (sk->sk_rcvbuf >> 2))
return;
} else {
size += up->forward_deficit;
--
2.13.5
^ permalink raw reply related
* [PATCH net] MAINTAINERS: Remove Yuval Mintz from maintainers list
From: aelior @ 2017-09-19 9:54 UTC (permalink / raw)
To: davem, aelior; +Cc: netdev, everest-linux-l2
From: Ariel Elior <aelior@cavium.com>
Remove Yuval from maintaining the bnx2x & qed* modules as he is no longer
working for the company. Thanks Yuval for your huge contributions and
tireless efforts over the many years and various companies.
Ariel
Signed-off-by: Ariel Elior <aelior@cavium.com>
---
MAINTAINERS | 2 --
1 file changed, 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 567343b..5d24dbf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2801,7 +2801,6 @@ S: Supported
F: drivers/scsi/bnx2i/
BROADCOM BNX2X 10 GIGABIT ETHERNET DRIVER
-M: Yuval Mintz <Yuval.Mintz@cavium.com>
M: Ariel Elior <ariel.elior@cavium.com>
M: everest-linux-l2@cavium.com
L: netdev@vger.kernel.org
@@ -10827,7 +10826,6 @@ S: Supported
F: drivers/scsi/qedi/
QLOGIC QL4xxx ETHERNET DRIVER
-M: Yuval Mintz <Yuval.Mintz@cavium.com>
M: Ariel Elior <Ariel.Elior@cavium.com>
M: everest-linux-l2@cavium.com
L: netdev@vger.kernel.org
--
2.9.4
^ permalink raw reply related
* [PATCH net-next 3/3] virtio-net: support XDP_REDIRECT
From: Jason Wang @ 2017-09-19 9:42 UTC (permalink / raw)
To: mst, jasowang, virtualization, netdev, linux-kernel; +Cc: John Fastabend
In-Reply-To: <1505814163-7315-1-git-send-email-jasowang@redhat.com>
This patch tries to add XDP_REDIRECT for virtio-net. The changes are
not complex as we could use exist XDP_TX helpers for most of the
work. The rest is passing the XDP_TX to NAPI handler for implementing
batching.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net. | 0
drivers/net/virtio_net.c | 77 ++++++++++++++++++++++++++++++++++++++----------
2 files changed, 62 insertions(+), 15 deletions(-)
create mode 100644 drivers/net/virtio_net.
diff --git a/drivers/net/virtio_net. b/drivers/net/virtio_net.
new file mode 100644
index 0000000..e69de29
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index db5924c..f6c1f13 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -29,6 +29,7 @@
#include <linux/slab.h>
#include <linux/cpu.h>
#include <linux/average.h>
+#include <linux/filter.h>
#include <net/route.h>
static int napi_weight = NAPI_POLL_WEIGHT;
@@ -372,8 +373,20 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
return skb;
}
-static bool virtnet_xdp_xmit(struct virtnet_info *vi,
- struct xdp_buff *xdp)
+static void virtnet_xdp_flush(struct net_device *dev)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ struct send_queue *sq;
+ unsigned int qp;
+
+ qp = vi->curr_queue_pairs - vi->xdp_queue_pairs + smp_processor_id();
+ sq = &vi->sq[qp];
+
+ virtqueue_kick(sq->vq);
+}
+
+static bool __virtnet_xdp_xmit(struct virtnet_info *vi,
+ struct xdp_buff *xdp)
{
struct virtio_net_hdr_mrg_rxbuf *hdr;
unsigned int len;
@@ -407,10 +420,19 @@ static bool virtnet_xdp_xmit(struct virtnet_info *vi,
return false;
}
- virtqueue_kick(sq->vq);
return true;
}
+static int virtnet_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct virtnet_info *vi = netdev_priv(dev);
+ bool sent = __virtnet_xdp_xmit(vi, xdp);
+
+ if (!sent)
+ return -ENOSPC;
+ return 0;
+}
+
static unsigned int virtnet_get_headroom(struct virtnet_info *vi)
{
return vi->xdp_queue_pairs ? VIRTIO_XDP_HEADROOM : 0;
@@ -483,7 +505,8 @@ static struct sk_buff *receive_small(struct net_device *dev,
struct virtnet_info *vi,
struct receive_queue *rq,
void *buf, void *ctx,
- unsigned int len)
+ unsigned int len,
+ bool *xdp_xmit)
{
struct sk_buff *skb;
struct bpf_prog *xdp_prog;
@@ -493,7 +516,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
unsigned int buflen = SKB_DATA_ALIGN(GOOD_PACKET_LEN + headroom) +
SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
struct page *page = virt_to_head_page(buf);
- unsigned int delta = 0;
+ unsigned int delta = 0, err;
struct page *xdp_page;
len -= vi->hdr_len;
@@ -541,8 +564,16 @@ static struct sk_buff *receive_small(struct net_device *dev,
delta = orig_data - xdp.data;
break;
case XDP_TX:
- if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
+ if (unlikely(!__virtnet_xdp_xmit(vi, &xdp)))
trace_xdp_exception(vi->dev, xdp_prog, act);
+ else
+ *xdp_xmit = true;
+ rcu_read_unlock();
+ goto xdp_xmit;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(dev, &xdp, xdp_prog);
+ if (!err)
+ *xdp_xmit = true;
rcu_read_unlock();
goto xdp_xmit;
default:
@@ -603,7 +634,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
struct receive_queue *rq,
void *buf,
void *ctx,
- unsigned int len)
+ unsigned int len,
+ bool *xdp_xmit)
{
struct virtio_net_hdr_mrg_rxbuf *hdr = buf;
u16 num_buf = virtio16_to_cpu(vi->vdev, hdr->num_buffers);
@@ -613,6 +645,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
struct bpf_prog *xdp_prog;
unsigned int truesize;
unsigned int headroom = mergeable_ctx_to_headroom(ctx);
+ int err;
head_skb = NULL;
@@ -678,12 +711,20 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
}
break;
case XDP_TX:
- if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
+ if (unlikely(!__virtnet_xdp_xmit(vi, &xdp)))
trace_xdp_exception(vi->dev, xdp_prog, act);
+ else
+ *xdp_xmit = true;
if (unlikely(xdp_page != page))
goto err_xdp;
rcu_read_unlock();
goto xdp_xmit;
+ case XDP_REDIRECT:
+ err = xdp_do_redirect(dev, &xdp, xdp_prog);
+ if (err)
+ *xdp_xmit = true;
+ rcu_read_unlock();
+ goto xdp_xmit;
default:
bpf_warn_invalid_xdp_action(act);
case XDP_ABORTED:
@@ -788,7 +829,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
}
static int receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
- void *buf, unsigned int len, void **ctx)
+ void *buf, unsigned int len, void **ctx, bool *xdp_xmit)
{
struct net_device *dev = vi->dev;
struct sk_buff *skb;
@@ -809,11 +850,11 @@ static int receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
}
if (vi->mergeable_rx_bufs)
- skb = receive_mergeable(dev, vi, rq, buf, ctx, len);
+ skb = receive_mergeable(dev, vi, rq, buf, ctx, len, xdp_xmit);
else if (vi->big_packets)
skb = receive_big(dev, vi, rq, buf, len);
else
- skb = receive_small(dev, vi, rq, buf, ctx, len);
+ skb = receive_small(dev, vi, rq, buf, ctx, len, xdp_xmit);
if (unlikely(!skb))
return 0;
@@ -1071,7 +1112,7 @@ static void refill_work(struct work_struct *work)
}
}
-static int virtnet_receive(struct receive_queue *rq, int budget)
+static int virtnet_receive(struct receive_queue *rq, int budget, bool *xdp_xmit)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
unsigned int len, received = 0, bytes = 0;
@@ -1083,13 +1124,13 @@ static int virtnet_receive(struct receive_queue *rq, int budget)
while (received < budget &&
(buf = virtqueue_get_buf_ctx(rq->vq, &len, &ctx))) {
- bytes += receive_buf(vi, rq, buf, len, ctx);
+ bytes += receive_buf(vi, rq, buf, len, ctx, xdp_xmit);
received++;
}
} else {
while (received < budget &&
(buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
- bytes += receive_buf(vi, rq, buf, len, NULL);
+ bytes += receive_buf(vi, rq, buf, len, NULL, xdp_xmit);
received++;
}
}
@@ -1161,15 +1202,19 @@ static int virtnet_poll(struct napi_struct *napi, int budget)
struct receive_queue *rq =
container_of(napi, struct receive_queue, napi);
unsigned int received;
+ bool xdp_xmit = false;
virtnet_poll_cleantx(rq);
- received = virtnet_receive(rq, budget);
+ received = virtnet_receive(rq, budget, &xdp_xmit);
/* Out of packets? */
if (received < budget)
virtqueue_napi_complete(napi, rq->vq, received);
+ if (xdp_xmit)
+ xdp_do_flush_map();
+
return received;
}
@@ -2069,6 +2114,8 @@ static const struct net_device_ops virtnet_netdev = {
.ndo_poll_controller = virtnet_netpoll,
#endif
.ndo_xdp = virtnet_xdp,
+ .ndo_xdp_xmit = virtnet_xdp_xmit,
+ .ndo_xdp_flush = virtnet_xdp_flush,
.ndo_features_check = passthru_features_check,
};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 2/3] virtio-net: add packet len average only when needed during XDP
From: Jason Wang @ 2017-09-19 9:42 UTC (permalink / raw)
To: mst, jasowang, virtualization, netdev, linux-kernel; +Cc: John Fastabend
In-Reply-To: <1505814163-7315-1-git-send-email-jasowang@redhat.com>
There's no need to add packet len average in the case of XDP_PASS
since it will be done soon after skb is created.
Cc: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a0ef4b0..db5924c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -656,6 +656,9 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
xdp.data_end = xdp.data + (len - vi->hdr_len);
act = bpf_prog_run_xdp(xdp_prog, &xdp);
+ if (act != XDP_PASS)
+ ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
+
switch (act) {
case XDP_PASS:
/* recalculate offset to account for any header
@@ -671,14 +674,12 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
put_page(page);
head_skb = page_to_skb(vi, rq, xdp_page,
offset, len, PAGE_SIZE);
- ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
return head_skb;
}
break;
case XDP_TX:
if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
trace_xdp_exception(vi->dev, xdp_prog, act);
- ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
if (unlikely(xdp_page != page))
goto err_xdp;
rcu_read_unlock();
@@ -690,7 +691,6 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
case XDP_DROP:
if (unlikely(xdp_page != page))
__free_pages(xdp_page, 0);
- ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
goto err_xdp;
}
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 1/3] virtio-net: remove unnecessary parameter of virtnet_xdp_xmit()
From: Jason Wang @ 2017-09-19 9:42 UTC (permalink / raw)
To: mst, jasowang, virtualization, netdev, linux-kernel; +Cc: John Fastabend
CC: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 511f833..a0ef4b0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -373,7 +373,6 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
}
static bool virtnet_xdp_xmit(struct virtnet_info *vi,
- struct receive_queue *rq,
struct xdp_buff *xdp)
{
struct virtio_net_hdr_mrg_rxbuf *hdr;
@@ -542,7 +541,7 @@ static struct sk_buff *receive_small(struct net_device *dev,
delta = orig_data - xdp.data;
break;
case XDP_TX:
- if (unlikely(!virtnet_xdp_xmit(vi, rq, &xdp)))
+ if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
trace_xdp_exception(vi->dev, xdp_prog, act);
rcu_read_unlock();
goto xdp_xmit;
@@ -677,7 +676,7 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
}
break;
case XDP_TX:
- if (unlikely(!virtnet_xdp_xmit(vi, rq, &xdp)))
+ if (unlikely(!virtnet_xdp_xmit(vi, &xdp)))
trace_xdp_exception(vi->dev, xdp_prog, act);
ewma_pkt_len_add(&rq->mrg_avg_pkt_len, len);
if (unlikely(xdp_page != page))
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 5/5] tls: Add generic NIC offload infrastructure.
From: Hannes Frederic Sowa @ 2017-09-19 9:40 UTC (permalink / raw)
To: Ilya Lesokhin
Cc: netdev@vger.kernel.org, davem@davemloft.net, davejwatson@fb.com,
tom@herbertland.com, Boris Pismenny, Aviad Yehezkel, Liran Liss
In-Reply-To: <AM4PR0501MB2723F6F5A22955102CDC2886D4600@AM4PR0501MB2723.eurprd05.prod.outlook.com>
Hello,
Ilya Lesokhin <ilyal@mellanox.com> writes:
> Hannes Frederic Sowa <hannes@stressinduktion.org> writes:
>
>> The user should be aware of that they can't migrate the socket to another
>> interface if they got hw offloaded. This is not the case for software offload.
>> Thus I think the user has to opt in and it shouldn't be a heuristic until we can
>> switch back to sw offload path.
>>
>> Maybe change flowi_oif to sk_bound_dev_if and somwhow lock it against
>> further changes if hw tls is in use?
>>
>
> I'm not sure I follow.
> We do set sk->sk_bound_dev_if to prevent further changes.
>
> Do you recommend we enable TLS offload only if SO_BINDTODEVICE
> was previously used on that socket?
> and prevent even users with CAP_NET_RAW from unbinding it?
>
> I would rather avoid requiring CAP_NET_RAW to use TLS offload.
> But admittedly I'm not sure setting sk->sk_bound_dev_if
> without CAP_NET_RAW like we do is legit either.
>
> Finally, the reason we made HW offload the default is that the user
> can use sudo ethtool -K enp0s4 tls-hw-tx-offload off to opt out of HW offload
> and we currently don't have anything equivalent for opting out of SW KTLS.
IMHO the decision if a TCP flow should be bounded to hw and thus never
push traffic to another interface should a decision the administrator
and the application should opt in. You might have your management
application which is accessible over multiple interfaces and your
production application which might want to use hw offloaded tls. Thus I
don't think only a single ethtool knob will do it.
I agree that SO_BINDTODEVICE is bad for this use case. First, the
CAP_NET_RAW limitation seems annoying and we don't want to enforce TLS
apps to have this capability. Second, the user space application doesn't
care which interface it should talk to (maybe?) but leave the routing
decision to the kernel and just opt in to TLS. SO_BINDTODEVICE doesn't
allow this.
sk_bound_dev_if can be rebound later with CAP_NET_RAW privileges, will
this be a problem?
Have you thought how the user space will configure the various
offloading features (sw, hw, none)? Will it in e.g. OpenSSL be part of
the Cipher Spec or will there be new functions around SSL_CTX to do so?
Maybe an enhancement of the TLS_TX setsockopt with a boolean for hw
offload is a solution?
Another question:
How is the dependency management done between socket layer and driver
layer? It seems a bit cyclic but judging from this code you don't hold
references to the device (dev_hold) (which is good, you don't want to
have users creating refs to devices). OTOH you somehow need to match
sockets from the device layer up to the socket. Will those be reference
counted or does that work without?
Thanks,
Hannes
^ permalink raw reply
* [Patch v3 3/3] ipv4: Namespaceify tcp_fastopen_blackhole_timeout knob
From: Haishuang Yan @ 2017-09-19 9:38 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Eric Dumazet, Wei Wang,
Luca BRUNO
Cc: netdev, linux-kernel, Haishuang Yan
In-Reply-To: <1505813896-12121-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
Different namespace application might require different time period in
second to disable Fastopen on active TCP sockets.
Tested:
Simulate following similar situation that the server's data gets dropped
after 3WHS.
C ---- syn-data ---> S
C <--- syn/ack ----- S
C ---- ack --------> S
S (accept & write)
C? X <- data ------ S
[retry and timeout]
And then print netstat of TCPFastOpenBlackhole, the counter increased as
expected when the firewall blackhole issue is detected and active TFO is
disabled.
# cat /proc/net/netstat | awk '{print $91}'
TCPFastOpenBlackhole
1
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
include/net/netns/ipv4.h | 3 +++
net/ipv4/sysctl_net_ipv4.c | 20 +++++++++++---------
net/ipv4/tcp_fastopen.c | 28 ++++++++++------------------
net/ipv4/tcp_ipv4.c | 2 ++
4 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 66b8335..d76edde 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -132,6 +132,9 @@ struct netns_ipv4 {
int sysctl_tcp_fastopen;
struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
spinlock_t tcp_fastopen_ctx_lock;
+ unsigned int sysctl_tcp_fastopen_blackhole_timeout;
+ atomic_t tfo_active_disable_times;
+ unsigned long tfo_active_disable_stamp;
#ifdef CONFIG_NET_L3_MASTER_DEV
int sysctl_udp_l3mdev_accept;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 20e19fe..cac8dd3 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -355,11 +355,13 @@ static int proc_tfo_blackhole_detect_timeout(struct ctl_table *table,
void __user *buffer,
size_t *lenp, loff_t *ppos)
{
+ struct net *net = container_of(table->data, struct net,
+ ipv4.sysctl_tcp_fastopen_blackhole_timeout);
int ret;
ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
if (write && ret == 0)
- tcp_fastopen_active_timeout_reset();
+ atomic_set(&net->ipv4.tfo_active_disable_times, 0);
return ret;
}
@@ -398,14 +400,6 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.proc_handler = proc_dointvec
},
{
- .procname = "tcp_fastopen_blackhole_timeout_sec",
- .data = &sysctl_tcp_fastopen_blackhole_timeout,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_tfo_blackhole_detect_timeout,
- .extra1 = &zero,
- },
- {
.procname = "tcp_abort_on_overflow",
.data = &sysctl_tcp_abort_on_overflow,
.maxlen = sizeof(int),
@@ -1083,6 +1077,14 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
.proc_handler = proc_tcp_fastopen_key,
},
+ {
+ .procname = "tcp_fastopen_blackhole_timeout_sec",
+ .data = &init_net.ipv4.sysctl_tcp_fastopen_blackhole_timeout,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_tfo_blackhole_detect_timeout,
+ .extra1 = &zero,
+ },
#ifdef CONFIG_IP_ROUTE_MULTIPATH
{
.procname = "fib_multipath_use_neigh",
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 21783c7..f3b00a9 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -407,25 +407,16 @@ bool tcp_fastopen_defer_connect(struct sock *sk, int *err)
* TFO connection with data exchanges.
*/
-/* Default to 1hr */
-unsigned int sysctl_tcp_fastopen_blackhole_timeout __read_mostly = 60 * 60;
-static atomic_t tfo_active_disable_times __read_mostly = ATOMIC_INIT(0);
-static unsigned long tfo_active_disable_stamp __read_mostly;
-
/* Disable active TFO and record current jiffies and
* tfo_active_disable_times
*/
void tcp_fastopen_active_disable(struct sock *sk)
{
- atomic_inc(&tfo_active_disable_times);
- tfo_active_disable_stamp = jiffies;
- NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENBLACKHOLE);
-}
+ struct net *net = sock_net(sk);
-/* Reset tfo_active_disable_times to 0 */
-void tcp_fastopen_active_timeout_reset(void)
-{
- atomic_set(&tfo_active_disable_times, 0);
+ atomic_inc(&net->ipv4.tfo_active_disable_times);
+ net->ipv4.tfo_active_disable_stamp = jiffies;
+ NET_INC_STATS(net, LINUX_MIB_TCPFASTOPENBLACKHOLE);
}
/* Calculate timeout for tfo active disable
@@ -434,17 +425,18 @@ void tcp_fastopen_active_timeout_reset(void)
*/
bool tcp_fastopen_active_should_disable(struct sock *sk)
{
- int tfo_da_times = atomic_read(&tfo_active_disable_times);
+ int tfo_da_times = atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times);
int multiplier;
unsigned long timeout;
+ unsigned int tfo_bh_timeout = sock_net(sk)->ipv4.sysctl_tcp_fastopen_blackhole_timeout;
if (!tfo_da_times)
return false;
/* Limit timout to max: 2^6 * initial timeout */
multiplier = 1 << min(tfo_da_times - 1, 6);
- timeout = multiplier * sysctl_tcp_fastopen_blackhole_timeout * HZ;
- if (time_before(jiffies, tfo_active_disable_stamp + timeout))
+ timeout = multiplier * tfo_bh_timeout * HZ;
+ if (time_before(jiffies, sock_net(sk)->ipv4.tfo_active_disable_stamp + timeout))
return true;
/* Mark check bit so we can check for successful active TFO
@@ -480,10 +472,10 @@ void tcp_fastopen_active_disable_ofo_check(struct sock *sk)
}
}
} else if (tp->syn_fastopen_ch &&
- atomic_read(&tfo_active_disable_times)) {
+ atomic_read(&sock_net(sk)->ipv4.tfo_active_disable_times)) {
dst = sk_dst_get(sk);
if (!(dst && dst->dev && (dst->dev->flags & IFF_LOOPBACK)))
- tcp_fastopen_active_timeout_reset();
+ atomic_set(&sock_net(sk)->ipv4.tfo_active_disable_times, 0);
dst_release(dst);
}
}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 29e042c..4b7aedb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2474,6 +2474,8 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
+ net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
+ atomic_set(&net->ipv4.tfo_active_disable_times, 0);
return 0;
fail:
--
1.8.3.1
^ permalink raw reply related
* [Patch v3 2/3] ipv4: Namespaceify tcp_fastopen_key knob
From: Haishuang Yan @ 2017-09-19 9:38 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Eric Dumazet, Wei Wang,
Luca BRUNO
Cc: netdev, linux-kernel, Haishuang Yan
In-Reply-To: <1505813896-12121-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
Different namespace application might require different tcp_fastopen_key
independently of the host.
Tested:
1. Container namespace:
# cat /proc/sys/net/ipv4/tcp_fastopen_key:
2817fff2-f803cf97-eadfd1f3-78c0992b
cookie key in tcp syn packets:
Fast Open Cookie
Kind: TCP Fast Open Cookie (34)
Length: 10
Fast Open Cookie: 1e5dd82a8c492ca9
2. Host:
# cat /proc/sys/net/ipv4/tcp_fastopen_key:
107d7c5f-68eb2ac7-02fb06e6-ed341702
cookie key in tcp syn packets:
Fast Open Cookie
Kind: TCP Fast Open Cookie (34)
Length: 10
Fast Open Cookie: e213c02bf0afbc8a
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
include/net/netns/ipv4.h | 4 ++++
include/net/tcp.h | 5 ++---
net/ipv4/af_inet.c | 2 +-
net/ipv4/sysctl_net_ipv4.c | 26 ++++++++++++------------
net/ipv4/tcp.c | 2 +-
net/ipv4/tcp_fastopen.c | 49 ++++++++++++++++++++++++++--------------------
net/ipv4/tcp_ipv4.c | 1 +
7 files changed, 49 insertions(+), 40 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index ce6dde0..66b8335 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -36,6 +36,8 @@ struct inet_timewait_death_row {
int sysctl_max_tw_buckets;
};
+struct tcp_fastopen_context;
+
struct netns_ipv4 {
#ifdef CONFIG_SYSCTL
struct ctl_table_header *forw_hdr;
@@ -128,6 +130,8 @@ struct netns_ipv4 {
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
int sysctl_tcp_fastopen;
+ struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
+ spinlock_t tcp_fastopen_ctx_lock;
#ifdef CONFIG_NET_L3_MASTER_DEV
int sysctl_udp_l3mdev_accept;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index f628967..f249c7e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1556,13 +1556,12 @@ struct tcp_fastopen_request {
};
void tcp_free_fastopen_req(struct tcp_sock *tp);
-extern struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
-int tcp_fastopen_reset_cipher(void *key, unsigned int len);
+int tcp_fastopen_reset_cipher(struct net *net, void *key, unsigned int len);
void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb);
struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct tcp_fastopen_cookie *foc);
-void tcp_fastopen_init_key_once(bool publish);
+void tcp_fastopen_init_key_once(struct net *net);
bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
struct tcp_fastopen_cookie *cookie);
bool tcp_fastopen_defer_connect(struct sock *sk, int *err);
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 309b849..5d4d9a3 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -222,7 +222,7 @@ int inet_listen(struct socket *sock, int backlog)
(tcp_fastopen & TFO_SERVER_ENABLE) &&
!inet_csk(sk)->icsk_accept_queue.fastopenq.max_qlen) {
fastopen_queue_tune(sk, backlog);
- tcp_fastopen_init_key_once(true);
+ tcp_fastopen_init_key_once(sock_net(sk));
}
err = inet_csk_listen_start(sk, backlog);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index e31e853c..20e19fe 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -251,10 +251,12 @@ static int proc_allowed_congestion_control(struct ctl_table *ctl,
return ret;
}
-static int proc_tcp_fastopen_key(struct ctl_table *ctl, int write,
+static int proc_tcp_fastopen_key(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp,
loff_t *ppos)
{
+ struct net *net = container_of(table->data, struct net,
+ ipv4.sysctl_tcp_fastopen);
struct ctl_table tbl = { .maxlen = (TCP_FASTOPEN_KEY_LENGTH * 2 + 10) };
struct tcp_fastopen_context *ctxt;
int ret;
@@ -265,7 +267,7 @@ static int proc_tcp_fastopen_key(struct ctl_table *ctl, int write,
return -ENOMEM;
rcu_read_lock();
- ctxt = rcu_dereference(tcp_fastopen_ctx);
+ ctxt = rcu_dereference(net->ipv4.tcp_fastopen_ctx);
if (ctxt)
memcpy(user_key, ctxt->key, TCP_FASTOPEN_KEY_LENGTH);
else
@@ -282,12 +284,7 @@ static int proc_tcp_fastopen_key(struct ctl_table *ctl, int write,
ret = -EINVAL;
goto bad_key;
}
- /* Generate a dummy secret but don't publish it. This
- * is needed so we don't regenerate a new key on the
- * first invocation of tcp_fastopen_cookie_gen
- */
- tcp_fastopen_init_key_once(false);
- tcp_fastopen_reset_cipher(user_key, TCP_FASTOPEN_KEY_LENGTH);
+ tcp_fastopen_reset_cipher(net, user_key, TCP_FASTOPEN_KEY_LENGTH);
}
bad_key:
@@ -401,12 +398,6 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.proc_handler = proc_dointvec
},
{
- .procname = "tcp_fastopen_key",
- .mode = 0600,
- .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
- .proc_handler = proc_tcp_fastopen_key,
- },
- {
.procname = "tcp_fastopen_blackhole_timeout_sec",
.data = &sysctl_tcp_fastopen_blackhole_timeout,
.maxlen = sizeof(int),
@@ -1085,6 +1076,13 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.mode = 0644,
.proc_handler = proc_dointvec,
},
+ {
+ .procname = "tcp_fastopen_key",
+ .mode = 0600,
+ .data = &init_net.ipv4.sysctl_tcp_fastopen,
+ .maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
+ .proc_handler = proc_tcp_fastopen_key,
+ },
#ifdef CONFIG_IP_ROUTE_MULTIPATH
{
.procname = "fib_multipath_use_neigh",
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dac56c4..23225c9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2749,7 +2749,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_FASTOPEN:
if (val >= 0 && ((1 << sk->sk_state) & (TCPF_CLOSE |
TCPF_LISTEN))) {
- tcp_fastopen_init_key_once(true);
+ tcp_fastopen_init_key_once(net);
fastopen_queue_tune(sk, val);
} else {
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index 9bd4ca5..21783c7 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -9,13 +9,18 @@
#include <net/inetpeer.h>
#include <net/tcp.h>
-struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
-
-static DEFINE_SPINLOCK(tcp_fastopen_ctx_lock);
-
-void tcp_fastopen_init_key_once(bool publish)
+void tcp_fastopen_init_key_once(struct net *net)
{
- static u8 key[TCP_FASTOPEN_KEY_LENGTH];
+ u8 key[TCP_FASTOPEN_KEY_LENGTH];
+ struct tcp_fastopen_context *ctxt;
+
+ rcu_read_lock();
+ ctxt = rcu_dereference(net->ipv4.tcp_fastopen_ctx);
+ if (ctxt) {
+ rcu_read_unlock();
+ return;
+ }
+ rcu_read_unlock();
/* tcp_fastopen_reset_cipher publishes the new context
* atomically, so we allow this race happening here.
@@ -23,8 +28,8 @@ void tcp_fastopen_init_key_once(bool publish)
* All call sites of tcp_fastopen_cookie_gen also check
* for a valid cookie, so this is an acceptable risk.
*/
- if (net_get_random_once(key, sizeof(key)) && publish)
- tcp_fastopen_reset_cipher(key, sizeof(key));
+ get_random_bytes(key, sizeof(key));
+ tcp_fastopen_reset_cipher(net, key, sizeof(key));
}
static void tcp_fastopen_ctx_free(struct rcu_head *head)
@@ -35,7 +40,7 @@ static void tcp_fastopen_ctx_free(struct rcu_head *head)
kfree(ctx);
}
-int tcp_fastopen_reset_cipher(void *key, unsigned int len)
+int tcp_fastopen_reset_cipher(struct net *net, void *key, unsigned int len)
{
int err;
struct tcp_fastopen_context *ctx, *octx;
@@ -59,26 +64,27 @@ int tcp_fastopen_reset_cipher(void *key, unsigned int len)
}
memcpy(ctx->key, key, len);
- spin_lock(&tcp_fastopen_ctx_lock);
+ spin_lock(&net->ipv4.tcp_fastopen_ctx_lock);
- octx = rcu_dereference_protected(tcp_fastopen_ctx,
- lockdep_is_held(&tcp_fastopen_ctx_lock));
- rcu_assign_pointer(tcp_fastopen_ctx, ctx);
- spin_unlock(&tcp_fastopen_ctx_lock);
+ octx = rcu_dereference_protected(net->ipv4.tcp_fastopen_ctx,
+ lockdep_is_held(&net->ipv4.tcp_fastopen_ctx_lock));
+ rcu_assign_pointer(net->ipv4.tcp_fastopen_ctx, ctx);
+ spin_unlock(&net->ipv4.tcp_fastopen_ctx_lock);
if (octx)
call_rcu(&octx->rcu, tcp_fastopen_ctx_free);
return err;
}
-static bool __tcp_fastopen_cookie_gen(const void *path,
+static bool __tcp_fastopen_cookie_gen(struct net *net,
+ const void *path,
struct tcp_fastopen_cookie *foc)
{
struct tcp_fastopen_context *ctx;
bool ok = false;
rcu_read_lock();
- ctx = rcu_dereference(tcp_fastopen_ctx);
+ ctx = rcu_dereference(net->ipv4.tcp_fastopen_ctx);
if (ctx) {
crypto_cipher_encrypt_one(ctx->tfm, foc->val, path);
foc->len = TCP_FASTOPEN_COOKIE_SIZE;
@@ -94,7 +100,8 @@ static bool __tcp_fastopen_cookie_gen(const void *path,
*
* XXX (TFO) - refactor when TCP_FASTOPEN_COOKIE_SIZE != AES_BLOCK_SIZE.
*/
-static bool tcp_fastopen_cookie_gen(struct request_sock *req,
+static bool tcp_fastopen_cookie_gen(struct net *net,
+ struct request_sock *req,
struct sk_buff *syn,
struct tcp_fastopen_cookie *foc)
{
@@ -102,7 +109,7 @@ static bool tcp_fastopen_cookie_gen(struct request_sock *req,
const struct iphdr *iph = ip_hdr(syn);
__be32 path[4] = { iph->saddr, iph->daddr, 0, 0 };
- return __tcp_fastopen_cookie_gen(path, foc);
+ return __tcp_fastopen_cookie_gen(net, path, foc);
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -110,13 +117,13 @@ static bool tcp_fastopen_cookie_gen(struct request_sock *req,
const struct ipv6hdr *ip6h = ipv6_hdr(syn);
struct tcp_fastopen_cookie tmp;
- if (__tcp_fastopen_cookie_gen(&ip6h->saddr, &tmp)) {
+ if (__tcp_fastopen_cookie_gen(net, &ip6h->saddr, &tmp)) {
struct in6_addr *buf = &tmp.addr;
int i;
for (i = 0; i < 4; i++)
buf->s6_addr32[i] ^= ip6h->daddr.s6_addr32[i];
- return __tcp_fastopen_cookie_gen(buf, foc);
+ return __tcp_fastopen_cookie_gen(net, buf, foc);
}
}
#endif
@@ -296,7 +303,7 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
goto fastopen;
if (foc->len >= 0 && /* Client presents or requests a cookie */
- tcp_fastopen_cookie_gen(req, skb, &valid_foc) &&
+ tcp_fastopen_cookie_gen(sock_net(sk), req, skb, &valid_foc) &&
foc->len == TCP_FASTOPEN_COOKIE_SIZE &&
foc->len == valid_foc.len &&
!memcmp(foc->val, valid_foc.val, foc->len)) {
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 88409b1..29e042c 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2473,6 +2473,7 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_timestamps = 1;
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
+ spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
return 0;
fail:
--
1.8.3.1
^ permalink raw reply related
* [Patch v3 1/3] ipv4: Namespaceify tcp_fastopen knob
From: Haishuang Yan @ 2017-09-19 9:38 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Eric Dumazet, Wei Wang,
Luca BRUNO
Cc: netdev, linux-kernel, Haishuang Yan
Different namespace application might require enable TCP Fast Open
feature independently of the host.
This patch series continues making more of the TCP Fast Open related
sysctl knobs be per net-namespace.
Reported-by: Luca BRUNO <lucab@debian.org>
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Change since v3:
* Split the patches to be per-sysctl
* Remove unrelated change by mistake
---
include/net/netns/ipv4.h | 1 +
include/net/tcp.h | 1 -
net/ipv4/af_inet.c | 7 ++++---
net/ipv4/sysctl_net_ipv4.c | 14 +++++++-------
net/ipv4/tcp.c | 4 ++--
net/ipv4/tcp_fastopen.c | 9 ++++-----
net/ipv4/tcp_ipv4.c | 2 ++
7 files changed, 20 insertions(+), 18 deletions(-)
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 20d061c..ce6dde0 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -127,6 +127,7 @@ struct netns_ipv4 {
int sysctl_tcp_timestamps;
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
+ int sysctl_tcp_fastopen;
#ifdef CONFIG_NET_L3_MASTER_DEV
int sysctl_udp_l3mdev_accept;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index b510f28..f628967 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -240,7 +240,6 @@
/* sysctl variables for tcp */
-extern int sysctl_tcp_fastopen;
extern int sysctl_tcp_retrans_collapse;
extern int sysctl_tcp_stdurg;
extern int sysctl_tcp_rfc1337;
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index e31108e..309b849 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -195,7 +195,7 @@ int inet_listen(struct socket *sock, int backlog)
{
struct sock *sk = sock->sk;
unsigned char old_state;
- int err;
+ int err, tcp_fastopen;
lock_sock(sk);
@@ -217,8 +217,9 @@ int inet_listen(struct socket *sock, int backlog)
* because the socket was in TCP_LISTEN state previously but
* was shutdown() rather than close().
*/
- if ((sysctl_tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) &&
- (sysctl_tcp_fastopen & TFO_SERVER_ENABLE) &&
+ tcp_fastopen = sock_net(sk)->ipv4.sysctl_tcp_fastopen;
+ if ((tcp_fastopen & TFO_SERVER_WO_SOCKOPT1) &&
+ (tcp_fastopen & TFO_SERVER_ENABLE) &&
!inet_csk(sk)->icsk_accept_queue.fastopenq.max_qlen) {
fastopen_queue_tune(sk, backlog);
tcp_fastopen_init_key_once(true);
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 0d3c038..e31e853c 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -401,13 +401,6 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.proc_handler = proc_dointvec
},
{
- .procname = "tcp_fastopen",
- .data = &sysctl_tcp_fastopen,
- .maxlen = sizeof(int),
- .mode = 0644,
- .proc_handler = proc_dointvec,
- },
- {
.procname = "tcp_fastopen_key",
.mode = 0600,
.maxlen = ((TCP_FASTOPEN_KEY_LENGTH * 2) + 10),
@@ -1085,6 +1078,13 @@ static int proc_tcp_available_ulp(struct ctl_table *ctl,
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "tcp_fastopen",
+ .data = &init_net.ipv4.sysctl_tcp_fastopen,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec,
+ },
#ifdef CONFIG_IP_ROUTE_MULTIPATH
{
.procname = "fib_multipath_use_neigh",
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5091402..dac56c4 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1126,7 +1126,7 @@ static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
struct sockaddr *uaddr = msg->msg_name;
int err, flags;
- if (!(sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
+ if (!(sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) ||
(uaddr && msg->msg_namelen >= sizeof(uaddr->sa_family) &&
uaddr->sa_family == AF_UNSPEC))
return -EOPNOTSUPP;
@@ -2759,7 +2759,7 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
case TCP_FASTOPEN_CONNECT:
if (val > 1 || val < 0) {
err = -EINVAL;
- } else if (sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
+ } else if (net->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_ENABLE) {
if (sk->sk_state == TCP_CLOSE)
tp->fastopen_connect = val;
else
diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
index e3c3322..9bd4ca5 100644
--- a/net/ipv4/tcp_fastopen.c
+++ b/net/ipv4/tcp_fastopen.c
@@ -9,8 +9,6 @@
#include <net/inetpeer.h>
#include <net/tcp.h>
-int sysctl_tcp_fastopen __read_mostly = TFO_CLIENT_ENABLE;
-
struct tcp_fastopen_context __rcu *tcp_fastopen_ctx;
static DEFINE_SPINLOCK(tcp_fastopen_ctx_lock);
@@ -282,18 +280,19 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
struct tcp_fastopen_cookie valid_foc = { .len = -1 };
bool syn_data = TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq + 1;
struct sock *child;
+ int tcp_fastopen = sock_net(sk)->ipv4.sysctl_tcp_fastopen;
if (foc->len == 0) /* Client requests a cookie */
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPFASTOPENCOOKIEREQD);
- if (!((sysctl_tcp_fastopen & TFO_SERVER_ENABLE) &&
+ if (!((tcp_fastopen & TFO_SERVER_ENABLE) &&
(syn_data || foc->len >= 0) &&
tcp_fastopen_queue_check(sk))) {
foc->len = -1;
return NULL;
}
- if (syn_data && (sysctl_tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
+ if (syn_data && (tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
goto fastopen;
if (foc->len >= 0 && /* Client presents or requests a cookie */
@@ -347,7 +346,7 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
return false;
}
- if (sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
+ if (sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
cookie->len = -1;
return true;
}
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index d9416b5..88409b1 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2472,6 +2472,8 @@ static int __net_init tcp_sk_init(struct net *net)
net->ipv4.sysctl_tcp_window_scaling = 1;
net->ipv4.sysctl_tcp_timestamps = 1;
+ net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
+
return 0;
fail:
tcp_sk_exit(net);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] VSOCK: fix uapi/linux/vm_sockets.h incomplete types
From: Stefan Hajnoczi @ 2017-09-18 15:21 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jhansen
In-Reply-To: <20170915.141432.1514866054657107786.davem@davemloft.net>
On Fri, Sep 15, 2017 at 02:14:32PM -0700, David Miller wrote:
> > diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
> > index b4ed5d895699..4ae5c625ac56 100644
> > --- a/include/uapi/linux/vm_sockets.h
> > +++ b/include/uapi/linux/vm_sockets.h
> > @@ -18,6 +18,10 @@
> >
> > #include <linux/socket.h>
> >
> > +#ifndef __KERNEL__
> > +#include <sys/socket.h> /* struct sockaddr */
> > +#endif
> > +
>
> There is no precedence whatsoever to include sys/socket.h in _any_ UAPI
> header file provided by the kernel.
<linux/if.h> does it for the same reason:
include/uapi/linux/if.h:#include <sys/socket.h> /* for struct sockaddr. */
If that is not acceptable for <linux/vm_sockets.h> then we could do the
same thing as <linux/in.h>:
#define __SOCK_SIZE__ 16 /* sizeof(struct sockaddr) */
Is that okay or do you have another suggestion for getting sizeof(struct
sockaddr)? It's needed to pad struct sockaddr_vm.
> __kernel_sa_family_t is what should be used in UAPI headers, only
> non-UAPI headers can use plain sa_family_t.
>
> So that is the correct fix for this problem.
Thanks, will fix in v2.
^ 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