* Re: [PATCH v7 bpf-next 06/10] tracepoint: compute num_args at build time
From: Mathieu Desnoyers @ 2018-03-28 13:49 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Linus Torvalds, Peter Zijlstra,
rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180328021105.4061744-7-ast@fb.com>
----- On Mar 27, 2018, at 10:11 PM, Alexei Starovoitov ast@fb.com wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> compute number of arguments passed into tracepoint
> at compile time and store it as part of 'struct tracepoint'.
> The number is necessary to check safety of bpf program access that
> is coming in subsequent patch.
Hi Alexei,
Given that only eBPF needs this parameter count, we can move
it to the struct bpf_raw_event_map newly introduced by Steven,
right ? This would reduce bloat of struct tracepoint. For instance,
we don't need to keep this count around when eBPF is configured
out.
Thanks,
Mathieu
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> include/linux/tracepoint-defs.h | 1 +
> include/linux/tracepoint.h | 12 ++++++------
> include/trace/define_trace.h | 14 +++++++-------
> 3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
> index 64ed7064f1fa..39a283c61c51 100644
> --- a/include/linux/tracepoint-defs.h
> +++ b/include/linux/tracepoint-defs.h
> @@ -33,6 +33,7 @@ struct tracepoint {
> int (*regfunc)(void);
> void (*unregfunc)(void);
> struct tracepoint_func __rcu *funcs;
> + u32 num_args;
> };
>
> #endif
> diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> index c94f466d57ef..c92f4adbc0d7 100644
> --- a/include/linux/tracepoint.h
> +++ b/include/linux/tracepoint.h
> @@ -230,18 +230,18 @@ extern void syscall_unregfunc(void);
> * structures, so we create an array of pointers that will be used for iteration
> * on the tracepoints.
> */
> -#define DEFINE_TRACE_FN(name, reg, unreg) \
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
> static const char __tpstrtab_##name[] \
> __attribute__((section("__tracepoints_strings"))) = #name; \
> struct tracepoint __tracepoint_##name \
> __attribute__((section("__tracepoints"))) = \
> - { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
> + { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
> static struct tracepoint * const __tracepoint_ptr_##name __used \
> __attribute__((section("__tracepoints_ptrs"))) = \
> &__tracepoint_##name;
>
> -#define DEFINE_TRACE(name) \
> - DEFINE_TRACE_FN(name, NULL, NULL);
> +#define DEFINE_TRACE(name, num_args) \
> + DEFINE_TRACE_FN(name, NULL, NULL, num_args);
>
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
> EXPORT_SYMBOL_GPL(__tracepoint_##name)
> @@ -275,8 +275,8 @@ extern void syscall_unregfunc(void);
> return false; \
> }
>
> -#define DEFINE_TRACE_FN(name, reg, unreg)
> -#define DEFINE_TRACE(name)
> +#define DEFINE_TRACE_FN(name, reg, unreg, num_args)
> +#define DEFINE_TRACE(name, num_args)
> #define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
> #define EXPORT_TRACEPOINT_SYMBOL(name)
>
> diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
> index d9e3d4aa3f6e..96b22ace9ae7 100644
> --- a/include/trace/define_trace.h
> +++ b/include/trace/define_trace.h
> @@ -25,7 +25,7 @@
>
> #undef TRACE_EVENT
> #define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_CONDITION
> #define TRACE_EVENT_CONDITION(name, proto, args, cond, tstruct, assign, print) \
> @@ -39,24 +39,24 @@
> #undef TRACE_EVENT_FN
> #define TRACE_EVENT_FN(name, proto, args, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef TRACE_EVENT_FN_COND
> #define TRACE_EVENT_FN_COND(name, proto, args, cond, tstruct, \
> assign, print, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT
> #define DEFINE_EVENT(template, name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_FN
> #define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
> - DEFINE_TRACE_FN(name, reg, unreg)
> + DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_PRINT
> #define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef DEFINE_EVENT_CONDITION
> #define DEFINE_EVENT_CONDITION(template, name, proto, args, cond) \
> @@ -64,7 +64,7 @@
>
> #undef DECLARE_TRACE
> #define DECLARE_TRACE(name, proto, args) \
> - DEFINE_TRACE(name)
> + DEFINE_TRACE(name, COUNT_ARGS(args))
>
> #undef TRACE_INCLUDE
> #undef __TRACE_INCLUDE
> --
> 2.9.5
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* [PATCH] tipc: avoid possible string overflow
From: Arnd Bergmann @ 2018-03-28 14:02 UTC (permalink / raw)
To: Jon Maloy, Ying Xue, David S. Miller
Cc: Arnd Bergmann, Parthasarathy Bhuvaragan, netdev, tipc-discussion,
linux-kernel
gcc points out that the combined length of the fixed-length inputs to
l->name is larger than the destination buffer size:
net/tipc/link.c: In function 'tipc_link_create':
net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region of size between 26 and 58 [-Werror=format-overflow=]
sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
^~ ~~~~~~~~
net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75) into a destination of size 60
sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
Using snprintf() ensures that the destination is still a nul-terminated
string in all cases. It's still theoretically possible that the string
gets trunctated though, so this patch should be carefully reviewed to
ensure that either truncation is impossible in practice, or that we're
ok with the truncation.
Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash values")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/tipc/link.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1289b4ba404f..c195ba036035 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -462,7 +462,8 @@ bool tipc_link_create(struct net *net, char *if_name, int bearer_id,
sprintf(peer_str, "%x", peer);
}
/* Peer i/f name will be completed by reset/activate message */
- sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
+ snprintf(l->name, sizeof(l->name), "%s:%s-%s:unknown",
+ self_str, if_name, peer_str);
strcpy(l->if_name, if_name);
l->addr = peer;
--
2.9.0
^ permalink raw reply related
* Re: [PATCH v6 bpf-next 08/11] bpf: introduce BPF_RAW_TRACEPOINT
From: Steven Rostedt @ 2018-03-28 14:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, David S. Miller, Daniel Borkmann,
Linus Torvalds, Peter Zijlstra, netdev, kernel-team, linux-api,
Kees Cook
In-Reply-To: <34b9892d-c091-7b83-d8e8-79c0ff813140@fb.com>
On Tue, 27 Mar 2018 17:51:55 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> Turned out it was in init.data section and got poisoned.
> this fixes it:
> @@ -258,6 +258,7 @@
> LIKELY_PROFILE() \
> BRANCH_PROFILE() \
> TRACE_PRINTKS() \
> + BPF_RAW_TP() \
> TRACEPOINT_STR()
>
> /*
> @@ -585,7 +586,6 @@
> *(.init.rodata) \
> FTRACE_EVENTS() \
> TRACE_SYSCALLS() \
> - BPF_RAW_TP() \
> KPROBE_BLACKLIST() \
> ERROR_INJECT_WHITELIST() \
> MEM_DISCARD(init.rodata) \
>
> and it works :)
> I will clean few other nits I found while debugging and respin.
Getting it properly working was an exercise left to the reader ;-)
Sorry about that, I did a bit of copy and paste to get it working, and
copied from code that did things a bit differently, so I massaged it by
hand, and doing it quickly as I had other things to work on.
-- Steve
^ permalink raw reply
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-28 14:14 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <alpine.DEB.2.20.1803272047260.8966@whs-18.cs.helsinki.fi>
On Wed, Mar 28, 2018 at 5:45 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
> On Tue, 27 Mar 2018, Yuchung Cheng wrote:
>
>> On Tue, Mar 27, 2018 at 7:23 AM, Ilpo Järvinen
>> <ilpo.jarvinen@helsinki.fi> wrote:
>> > On Mon, 26 Mar 2018, Yuchung Cheng wrote:
>> >
>> >> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
>> >> <ilpo.jarvinen@helsinki.fi> wrote:
>> >> >
>> >> > A miscalculation for the number of acknowledged packets occurs during
>> >> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
>> >> > any non-retransmitted skbs. The reason is that pkts_acked value
>> >> > calculated in tcp_clean_rtx_queue is not correct for slow start after
>> >> > RTO as it may include segments that were not lost and therefore did
>> >> > not need retransmissions in the slow start following the RTO. Then
>> >> > tcp_slow_start will add the excess into cwnd bloating it and
>> >> > triggering a burst.
>> >> >
>> >> > Instead, we want to pass only the number of retransmitted segments
>> >> > that were covered by the cumulative ACK (and potentially newly sent
>> >> > data segments too if the cumulative ACK covers that far).
>> >> >
>> >> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
>> >> > ---
>> >>
>> >> My understanding is there are two problems
>> >>
>> >> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
>> >> precise cumulatively acked count, not newly acked count?
>> >
>> > While I'm not entirely sure if you intented to say that my fix is broken
>> > or not, I thought this very difference alot while making the fix and I
>> > believe that this fix is needed because of the discontinuity at RTO
>> > (sacked_out is cleared as we set L-bits + lost_out). This is an artifact
>> > in the imitation of sacked_out for non-SACK but at RTO we can't keep that
>> > in sync because we set L-bits (and have no S-bits to guide us). Thus, we
>> > cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
>> >
>> > In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
>> > tp->delivered, using plain cumulative acked causes congestion control
>> > breakage later as call to tcp_cong_control will directly use the
>> > difference in tp->delivered.
>> >
>> > This boils down the exact definition of tp->delivered (the one given in
>> > the header is not detailed enough). I guess you might have better idea
>> > what it exactly is since one of you has added it? There are subtle things
>> > in the defination that can make it entirely unsuitable for cc decisions.
>> > Should those segments that we (possibly) already counted into
>> > tp->delivered during (potentially preceeding) CA_Recovery be added to it
>> > for _second time_ or not? This fix avoids such double counting (the
>> Where is the double counting, assuming normal DUPACK behavior?
>>
>> In the non-sack case:
>>
>> 1. upon receiving a DUPACK, we assume one packet has been delivered by
>> incrementing tp->delivered in tcp_add_reno_sack()
>
> 1b. RTO here. We clear tp->sacked_out at RTO (i.e., the discontinuity
> I've tried to point out quite many times already)...
>
>> 2. upon receiving a partial ACK or an ACK that acks recovery point
>> (high_seq), tp->delivered is incremented by (cumulatively acked -
>> #dupacks) in tcp_remove_reno_sacks()
>
> ...and this won't happen correctly anymore after RTO (since non-SACK
> won't keep #dupacks due to the discontinuity). Thus we end up adding
> cumulatively acked - 0 to tp->delivered on those ACKs.
>
>> therefore tp->delivered is tracking the # of packets delivered
>> (sacked, acked, DUPACK'd) with the most information it could have
>> inferred.
>
> Since you didn't answer any of my questions about tp->delivered directly,
> let me rephrase them to this example (non-SACK, of course):
>
> 4 segments outstanding. RTO recovery underway (lost_out=4, sacked_out=0).
> Cwnd = 2 so the sender rexmits 2 out of 4. We get cumulative ACK for
> three segments. How much should tp->delivered be incremented? 2 or 3?
>
> ...I think 2 is the right answer.
>
>> From congestion control's perspective, it cares about the delivery
>> information (e.g. how much), not the sequences (what or how).
>
> I guess you must have missed my point. I'm talking about "how much"
> whole the time. It's just about when can we account for it (and when not).
>
>> I am pointing out that
>>
>> 1) your fix may fix one corner CC packet accounting issue in the
>> non-SACK and CA_Loss
>> 2) but it does not fix the other (major) CC packet accounting issue in
>> the SACK case
>
> You say major but it's major if and only if one is using one of the
> affected cc modules which were not that many and IMHO not very mainstream
> anyway (check my list from the previous email, not that I'd mind if they'd
> get fixed too). The rest of the cc modules do not seem to use the incorrect
> value even if some of them have the pkts_acked callback.
>
> Other CC packet accounting (regardless of SACK) is based on tp->delivered
> and tp->delivered is NOT similarly miscounted with SACK because the logic
> depend on !TCPCB_SACKED_ACKED (that fails only if we have very high ACK
> loss).
>
>> 3) it breaks the dynamic dupthresh / reordering in the non-SACK case
>> as tcp_check_reno_reordering requires strictly cum. ack.
>
> I think that the current non-SACK reordering detection logic is not that
> sound after RTO (but I'm yet to prove this). ...There seems to be some
> failure modes which is why I even thought of disabling the whole thing
> for non-SACK RTO recoveries and as it now seems you do care more about
> non-SACK than you initial claimed, I might even have motivation to fix
> more those corners rather than the worst bugs only ;-). ...But I'll make
> the tp->delivered fix only in this patch to avoid any change this part of
> the code.
>
>> Therefore I prefer
>> a) using tp->delivered to fix (1)(2)
>> b) perhaps improving tp->delivered SACK emulation code in the non-SACK case
>
> Somehow I get an impression that you might assume/say here that
[resending in plaintext]
That's wrong impression. Perhaps it's worth re-iterating what I agree
and disagree
1. [agree] there's accounting issue in non-SACK as you discovered
which causes CC misbehavior
2. [major disagree] adjusting pkts_acked for ca_ops->pkts_acked in non-sack
=> that field is not used by common C.C. (you said so too)
3. [disagree] adjusting pkts_acked may not affect reordering
accounting in non-sack
For cubic or reno, the main source is the "acked_sacked" passed into
tcp_cong_avoid(). that variable is derived from tp->delivered.
Therefore we need to fix that to address the problem in (1)
I have yet to read your code. Will read later today.
> tp->delivered is all correct for non-SACK? ...It isn't without a fix.
> Therefore a) won't help any for non-SACK. Tp->delivered for non-SACK is
> currently (mis!)calculated in tcp_remove_reno_sacks because the incorrect
> pkts_acked is fed to it. ...Thus, b) is an intermediate step in the
> miscalculation chain I'm fixing with this change. B) resolves also (1)
> without additional changes to logic!
>
> I've included below the updated change that only fixes tp->delivered
> calculation (not tested besides compiling). ...But I think it's worse than
> my previous version because tcp_remove_reno_sacks then assumes
> non-sensical L|S skbs (but there seem to be other, worse limitations in
> sacked_out imitation after RTO because we've all skbs marked with L-bit so
> it's not that big deal for me as most of the that imitation code is no-op
> anyway after RTO).
>
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 9a1b3c1..e11748d 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -1868,7 +1868,6 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
>
> if (acked > 0) {
> /* One ACK acked hole. The rest eat duplicate ACKs. */
> - tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> if (acked - 1 >= tp->sacked_out)
> tp->sacked_out = 0;
> else
> @@ -1878,6 +1877,12 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> tcp_verify_left_out(tp);
> }
>
> +static void tcp_update_reno_delivered(struct tcp_sock *tp, int acked)
> +{
> + if (acked > 0)
> + tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> +}
> +
> static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
> {
> tp->sacked_out = 0;
> @@ -3027,6 +3032,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> long seq_rtt_us = -1L;
> long ca_rtt_us = -1L;
> u32 pkts_acked = 0;
> + u32 rexmit_acked = 0;
> + u32 newdata_acked = 0;
> u32 last_in_flight = 0;
> bool rtt_update;
> int flag = 0;
> @@ -3056,8 +3063,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (unlikely(sacked & TCPCB_RETRANS)) {
> - if (sacked & TCPCB_SACKED_RETRANS)
> + if (sacked & TCPCB_SACKED_RETRANS) {
> tp->retrans_out -= acked_pcount;
> + rexmit_acked += acked_pcount;
> + }
> flag |= FLAG_RETRANS_DATA_ACKED;
> } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> last_ackt = skb->skb_mstamp;
> @@ -3070,6 +3079,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> reord = start_seq;
> if (!after(scb->end_seq, tp->high_seq))
> flag |= FLAG_ORIG_SACK_ACKED;
> + else
> + newdata_acked += acked_pcount;
> }
>
> if (sacked & TCPCB_SACKED_ACKED) {
> @@ -3151,6 +3162,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (tcp_is_reno(tp)) {
> + tcp_update_reno_delivered(tp, icsk->icsk_ca_state != TCP_CA_Loss ?
> + pkts_acked :
> + rexmit_acked + newdata_acked);
> tcp_remove_reno_sacks(sk, pkts_acked);
> } else {
> int delta;
^ permalink raw reply
* [PATCH] [net-next] sctp: fix unused lable warning
From: Arnd Bergmann @ 2018-03-28 14:14 UTC (permalink / raw)
To: Vlad Yasevich, Neil Horman, David S. Miller
Cc: Arnd Bergmann, Al Viro, linux-sctp, netdev, linux-kernel
The proc file cleanup left a label possibly unused:
net/sctp/protocol.c: In function 'sctp_defaults_init':
net/sctp/protocol.c:1304:1: error: label 'err_init_proc' defined but not used [-Werror=unused-label]
This adds an #ifdef around it to match the respective 'goto'.
Fixes: d47d08c8ca05 ("sctp: use proc_remove_subtree()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/sctp/protocol.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index c1c64b78c39d..d685f8456762 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -1301,8 +1301,10 @@ static int __net_init sctp_defaults_init(struct net *net)
return 0;
+#ifdef CONFIG_PROC_FS
err_init_proc:
cleanup_sctp_mibs(net);
+#endif
err_init_mibs:
sctp_sysctl_net_unregister(net);
err_sysctl_register:
--
2.9.0
^ permalink raw reply related
* Re: Problems: network rx out-of-order issue
From: Eric Dumazet @ 2018-03-28 14:19 UTC (permalink / raw)
To: Anny Hu, netdev; +Cc: zhen.jiang
In-Reply-To: <1522229201.8705.44.camel@mbjswglx818>
On 03/28/2018 02:26 AM, Anny Hu wrote:
> Dears,
>
> Recently, we find the following patch will impact multi-core network throughput performance on kernel-4.9,
> for it will cause rx packet out-of-order.
> commit id: 4cd13c21b207e80ddb1144c576500098f2d5f882
>
> [kernel version]:
> kernel-4.9
>
> [repeat steps]
> 1. two our phones with load based on kernel-4.9, enable wifi and set up P2P connection;
> 2. two reference phones with load based on kernel-4.4, enable wifi and set up P2P connection;
> 3. do iperf UDP downlink test with the following commands;
> Client: adb shell iperf -c 192.168.49.xx -u -i 1 -t 60 -b 15M
> Server: adb shell iperf -s -u
> 4. out-of-order rate of our phones are obviously higher than reference phone:
> DUT Lost rate out of order
> Our phone 0.011% 164
> Reference phone 0.0091% 1
>
> [Code flow]
> Wi-Fi driver trigger rx_thread to receive rx packets and call netif_rx_ni() to push packets to tcp stack in thread context.
>
> [netlog]:
> No Time Source ip Dest ip Protocol Len Ip id Info
> 3811 2010-01-01 08:11:35.075522 192.168.43.1 192.168.43.90 UDP 144 0x25ee (9710) 60834 -> 5201 Len=100
> 3812 2010-01-01 08:11:35.075805 192.168.43.1 192.168.43.90 UDP 144 0x25f5 (9717) 60834 -> 5201 Len=100
> 3813 2010-01-01 08:11:35.075549 192.168.43.1 192.168.43.90 UDP 144 0x25ef (9711) 60834 -> 5201 Len=100
> 3814 2010-01-01 08:11:35.075944 192.168.43.1 192.168.43.90 UDP 144 0x25f6 (9718) 60834 -> 5201 Len=100
> 3815 2010-01-01 08:11:35.075567 192.168.43.1 192.168.43.90 UDP 144 0x25f0 (9712) 60834 -> 5201 Len=100
>
> out-of-order issue happens on:
> 1) ip_id=0x25f5 is received before ip_id=0x25ef, which are belong to the same downlink connection;
> 2) ip_id=0x25f6 is received before ip_id=0x25f0, which are belong to the same downlink connection.
>
>
> [kernel log]:
> We add debug log in rx kernel flow, such as netif_rx_ni() -> enqueue_to_backlog -> do_softirq() -> net_rx_action() -> process_backlog()
>
> kernel time
> | cpu id which process running on
> | | process name function name
> | | | |
> [ 598.837358] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ee, pending=0x8a, backlog_state=1
>
> // wifi rx_thread is running on cpu7 to receive rx packets of ip_id 0x25ee ~ 0x25f4, which are only enqueued.
> [ 598.837366] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ee, input_qlen=1, proc_qlen=0, pending=0x8a, backlog_state=1
>
> // because ksoftirqd is in running queue, do_softirq() exit and net_rx_action isn't executed actually.
>
> [ 598.837373] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> [ 598.837383] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25ef,input_qlen=2, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837391] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0 pending=0x8a, ksoftirqd_running=1
> [ 598.837400] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f0,input_qlen=3, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837407] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837417] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f1,input_qlen=4, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837424] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837434] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f2,input_qlen=5, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837441] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
> [ 598.837450] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f3,input_qlen=6, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837458] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> // after ip_id(0x25f4), there is no rx packet in this wifi interrupt, so rx_thread exit.
> [ 598.837470] (7)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f4,input_qlen=7, proc_qlen=0, pending=0x8a, backlog_state=1
> [ 598.837479] (7)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8a, ksoftirqd_running=1
>
> // ksoftirqd is scheduled to run and call do_softirq(), so net_rx_action() is executed.
> [ 598.837511] (7)[45:ksoftirqd/7]: [RX]net_rx_action: budget=300, time=4295042005
> // net_rx_action() call process_backlog() to handle rx packet of ip_id=0x25ee
>
> [ 598.837646] (7)[45:ksoftirqd/7]: [RX]process_backlog: ip_id=0x25ee, delay=126308, work=1, input_qlen=0, proc_qlen=6
>
> // wifi interrupt happens and trigger rx_thread to receive the next rx packet, which is running on cpu1 this time.
> [ 598.837660] (1)[3282:rx_thread]: [RX]enqueue_to_backlog: ip_id=0x25f5, input_qlen=1, proc_qlen=0, pending=0x8, backlog_state=1
> // because ksoftirqd is not running, net_rx softirq is executed immediately.
> [ 598.837666] (1)[3282:rx_thread]: [RX]do_softirq: preempt=0x1, in_interrupt()=0x0, pending=0x8, ksoftirqd_running =0
>
> // so ksoftirqd on cpu7 and rx_thread on cpu1 are running at the same time.
> [ 598.837679] (1)[3282:rx_thread]: [RX]net_rx_action: budget=300, time=4295042005
>
> // ip_id=0x25ef is handled on cpu7 by net_rx softirq in ksoftirqd() thread context.
> [ 598.837762] (7)[45:ksoftirqd/7]: [RX]process_backlog: ip_id=0x25ef, delay=70385, work=2, input_qlen=0, proc_qlen=5
>
> // ip_id=0x25f5 is handled on cpu1 by net_rx softirq in rx_thread context, out-of-order issue happens.
>
> [ 598.837768] (1)[3282:rx_thread]: [RX]process_backlog: ip_id=0x25f5, delay=82000, work=1, quota=64, input_qlen=0, proc_qlen=0
>
> // ip_id=0x25f0 is handled on cpu7 by net_rx softirq in ksoftirqd() thread context.
> [ 598.837878] (7)[45:ksoftirqd/7]: [RX]process_backlog:ip_id=0x25f0, delay=47539, work=3, input_qlen=0, proc_qlen=4
>
>
> Could you please help to check if this out-of-order issue is caused by kernel-4.9 patch above?
>
> Thanks very much!
>
A driver calling netif_rx_ni() can have out of order packets,
regardless of commit 4cd13c21b207e80ddb1144c57650
This patch has changed scheduling, and might have pointed out ancient bugs/issues.
'thread context' means cpu can change under us (preempt kernels)
Which driver are you using exactly ?
^ permalink raw reply
* Re: [bpf-next PATCH v2 3/4] bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
From: Daniel Borkmann @ 2018-03-28 14:21 UTC (permalink / raw)
To: John Fastabend, ast; +Cc: netdev, davem
In-Reply-To: <20180327172322.11354.54016.stgit@john-Precision-Tower-5810>
On 03/27/2018 07:23 PM, John Fastabend wrote:
> Add support for the BPF_F_INGRESS flag in skb redirect helper. To
> do this convert skb into a scatterlist and push into ingress queue.
> This is the same logic that is used in the sk_msg redirect helper
> so it should feel familiar.
>
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> ---
> include/linux/filter.h | 1 +
> kernel/bpf/sockmap.c | 94 +++++++++++++++++++++++++++++++++++++++---------
> net/core/filter.c | 2 +
> 3 files changed, 78 insertions(+), 19 deletions(-)
[...]
> if (!sg->length && md->sg_start == md->sg_end) {
> list_del(&md->list);
> + if (md->skb)
> + consume_skb(md->skb);
> kfree(md);
> }
> }
> @@ -1045,27 +1048,72 @@ static int smap_verdict_func(struct smap_psock *psock, struct sk_buff *skb)
> __SK_DROP;
> }
>
> +static int smap_do_ingress(struct smap_psock *psock, struct sk_buff *skb)
> +{
> + struct sock *sk = psock->sock;
> + int copied = 0, num_sg;
> + struct sk_msg_buff *r;
> +
> + r = kzalloc(sizeof(struct sk_msg_buff), __GFP_NOWARN | GFP_ATOMIC);
> + if (unlikely(!r))
> + return -EAGAIN;
> +
> + if (!sk_rmem_schedule(sk, skb, skb->len)) {
> + kfree(r);
> + return -EAGAIN;
> + }
> + sk_mem_charge(sk, skb->len);
Usually mem accounting is based on truesize. This is not done here since
you need the exact length of the skb for the sg list later on, right?
> + sg_init_table(r->sg_data, MAX_SKB_FRAGS);
> + num_sg = skb_to_sgvec(skb, r->sg_data, 0, skb->len);
> + if (unlikely(num_sg < 0)) {
> + kfree(r);
Don't we need to undo the mem charge here in case of error?
> + return num_sg;
> + }
> + copied = skb->len;
> + r->sg_start = 0;
> + r->sg_end = num_sg == MAX_SKB_FRAGS ? 0 : num_sg;
> + r->skb = skb;
> + list_add_tail(&r->list, &psock->ingress);
> + sk->sk_data_ready(sk);
> + return copied;
> +}
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Rob Herring @ 2018-03-28 14:31 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vicenţiu Galanopulo, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mark.rutland@arm.com,
davem@davemloft.net, marcel@holtmann.org,
devicetree@vger.kernel.org, Madalin-cristian Bucur,
Alexandru Marginean
In-Reply-To: <20180327142408.GL5862@lunn.ch>
On Tue, Mar 27, 2018 at 9:24 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>> > This is a 2nd MDIO address, right? Can't you just append this to reg property?
>
> Hi Rob
>
> It is a sub address.
>
> There are two different MDIO addressing schemes. Clause 22 allowed for
> 32 different addresses on an MDIO bus. Clause 45 extended that. You
> have the existing 32 addresses for a package. However, within a
> package, you can have 32 devices.
Sounds similar to functions in PCI land (which are part of reg address).
> You are supposed to be able to look are registers inside the package,
> and it will tell you which devices in the packages are in use. You can
> then look at those devices and figure out what they are using ID
> registers.
>
> However some vendors get this wrong, they don't fill in the devices in
> package information. So the generic probe code never finds them. We
> need to pass it a hint, go looking at this specific device in the
> package.
If this is a rare case and something future devices should get right,
then I'm more inclined to use 'dev-addr' rather than extending reg.
> You can mix Clause 22 and Clause 45 on the same bus. Does DT allow two
> different reg formats to be used at once? Can we have some reg
> properties with a single value, and some with two values? I thought
> #address-cells = <1> means there should be a single address in reg.
#address-cells is how many cells (aka u32) it takes to store an
address, not how many addresses you have. The number of addresses is
(sizeof(reg) / 4) / (#address-cells + #size-cells). So yes, you can
have different number of addresses for each device, but you can't have
different sizes of addresses (e.g. 32-bit and 64-bit) in one bus. But
I think in this case it would logically be 1 address with 2 cells
because it is the port and device together that make up the address.
If you did that, you'd have to define how to express a clause 22
device with 2 cells. You could either set a high bit in the first cell
to indicate clause 45 address or use an illegal device address in the
2nd cell.
However, the MDIO core would need to handle 2 address cells. If more
than 1 address cell is an error currently, that causes a compatibility
problem with new dtb and older kernels (but could be addressed with
stable updates).
Rob
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: David Miller @ 2018-03-28 14:38 UTC (permalink / raw)
To: eric.dumazet; +Cc: krzysztof.blaszkowski, netdev, gregkh
In-Reply-To: <14343877-a2a0-215e-e052-2dfb7e4c4b09@gmail.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 28 Mar 2018 06:38:21 -0700
> https://patchwork.ozlabs.org/patch/886324/
I have this in my current -stable submission set, and I'm working
actively on this right now.
Thanks.
^ permalink raw reply
* RE: [PATCH] tipc: avoid possible string overflow
From: Jon Maloy @ 2018-03-28 14:41 UTC (permalink / raw)
To: Arnd Bergmann, Ying Xue, David S. Miller
Cc: Parthasarathy Bhuvaragan, netdev@vger.kernel.org,
tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org
In-Reply-To: <20180328140302.2594031-1-arnd@arndb.de>
> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Wednesday, March 28, 2018 10:02
> To: Jon Maloy <jon.maloy@ericsson.com>; Ying Xue
> <ying.xue@windriver.com>; David S. Miller <davem@davemloft.net>
> Cc: Arnd Bergmann <arnd@arndb.de>; Parthasarathy Bhuvaragan
> <parthasarathy.bhuvaragan@ericsson.com>; netdev@vger.kernel.org; tipc-
> discussion@lists.sourceforge.net; linux-kernel@vger.kernel.org
> Subject: [PATCH] tipc: avoid possible string overflow
>
> gcc points out that the combined length of the fixed-length inputs to
> l->name is larger than the destination buffer size:
>
> net/tipc/link.c: In function 'tipc_link_create':
> net/tipc/link.c:465:26: error: '%s' directive writing up to 32 bytes into a region
> of size between 26 and 58 [-Werror=format-overflow=]
> sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
> ^~ ~~~~~~~~
> net/tipc/link.c:465:2: note: 'sprintf' output 11 or more bytes (assuming 75)
> into a destination of size 60
> sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
>
> Using snprintf() ensures that the destination is still a nul-terminated string in
> all cases. It's still theoretically possible that the string gets trunctated though,
> so this patch should be carefully reviewed to ensure that either truncation is
> impossible in practice, or that we're ok with the truncation.
Theoretically, maximum bearer name is MAX_BEARER_NAME - 3 = 29 (because if_name is only the part after the ":" in a bearer name, and is zero-terminated.
The lines just above in the code reveals that the maximum length of self_str and peer_str is 16.
This taken together means that the theoretically max length of a link name becomes:
16 + 1 + 29 + 1 + 16 + 1 + 29 = 93. Since we also need room for a terminating zero, we need to extend the tipc_link::name array to 96 bytes.
I'll fix that.
Thank you to for reporting this.
///jon
>
> Fixes: 25b0b9c4e835 ("tipc: handle collisions of 32-bit node address hash
> values")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/tipc/link.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/tipc/link.c b/net/tipc/link.c index 1289b4ba404f..c195ba036035
> 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -462,7 +462,8 @@ bool tipc_link_create(struct net *net, char *if_name,
> int bearer_id,
> sprintf(peer_str, "%x", peer);
> }
> /* Peer i/f name will be completed by reset/activate message */
> - sprintf(l->name, "%s:%s-%s:unknown", self_str, if_name, peer_str);
> + snprintf(l->name, sizeof(l->name), "%s:%s-%s:unknown",
> + self_str, if_name, peer_str);
>
> strcpy(l->if_name, if_name);
> l->addr = peer;
> --
> 2.9.0
^ permalink raw reply
* [PATCH net-next] net: Call add/kill vid ndo on vlan filter feature toggling
From: Gal Pressman @ 2018-03-28 14:46 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, Tariq Toukan, Gal Pressman
NETIF_F_HW_VLAN_[CS]TAG_FILTER features require more than just a bit
flip in dev->features in order to keep the driver in a consistent state.
These features notify the driver of each added/removed vlan, but toggling
of vlan-filter does not notify the driver accordingly for each of the
existing vlans.
This patch implements a similar solution to NETIF_F_RX_UDP_TUNNEL_PORT
behavior (which notifies the driver about UDP ports in the same manner
that vids are reported).
Each toggling of the features propagates to the 8021q module, which
iterates over the vlans and call add/kill ndo accordingly.
Signed-off-by: Gal Pressman <galp@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
include/linux/if_vlan.h | 24 +++++++++++
include/linux/netdevice.h | 4 ++
net/8021q/vlan.c | 21 ++++++++++
net/8021q/vlan.h | 3 ++
net/8021q/vlan_core.c | 101 ++++++++++++++++++++++++++++++++++------------
net/core/dev.c | 20 +++++++++
6 files changed, 148 insertions(+), 25 deletions(-)
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index c4a1cff..24d1976 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -83,6 +83,30 @@ static inline bool is_vlan_dev(const struct net_device *dev)
#define skb_vlan_tag_get_id(__skb) ((__skb)->vlan_tci & VLAN_VID_MASK)
#define skb_vlan_tag_get_prio(__skb) ((__skb)->vlan_tci & VLAN_PRIO_MASK)
+static inline int vlan_get_rx_ctag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ return notifier_to_errno(call_netdevice_notifiers(NETDEV_CVLAN_FILTER_PUSH_INFO, dev));
+}
+
+static inline void vlan_drop_rx_ctag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_CVLAN_FILTER_DROP_INFO, dev);
+}
+
+static inline int vlan_get_rx_stag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ return notifier_to_errno(call_netdevice_notifiers(NETDEV_SVLAN_FILTER_PUSH_INFO, dev));
+}
+
+static inline void vlan_drop_rx_stag_filter_info(struct net_device *dev)
+{
+ ASSERT_RTNL();
+ call_netdevice_notifiers(NETDEV_SVLAN_FILTER_DROP_INFO, dev);
+}
+
/**
* struct vlan_pcpu_stats - VLAN percpu rx/tx stats
* @rx_packets: number of received packets
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 2a2d9cf..da44dab 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2349,6 +2349,10 @@ enum netdev_cmd {
NETDEV_UDP_TUNNEL_PUSH_INFO,
NETDEV_UDP_TUNNEL_DROP_INFO,
NETDEV_CHANGE_TX_QUEUE_LEN,
+ NETDEV_CVLAN_FILTER_PUSH_INFO,
+ NETDEV_CVLAN_FILTER_DROP_INFO,
+ NETDEV_SVLAN_FILTER_PUSH_INFO,
+ NETDEV_SVLAN_FILTER_DROP_INFO,
};
const char *netdev_cmd_to_name(enum netdev_cmd cmd);
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index bd0ed39..4b33728 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -360,6 +360,7 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
struct vlan_dev_priv *vlan;
bool last = false;
LIST_HEAD(list);
+ int err;
if (is_vlan_dev(dev)) {
int err = __vlan_device_event(dev, event);
@@ -489,6 +490,26 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
vlan_group_for_each_dev(grp, i, vlandev)
call_netdevice_notifiers(event, vlandev);
break;
+
+ case NETDEV_CVLAN_FILTER_PUSH_INFO:
+ err = vlan_filter_push_vids(vlan_info, htons(ETH_P_8021Q));
+ if (err)
+ return notifier_from_errno(err);
+ break;
+
+ case NETDEV_CVLAN_FILTER_DROP_INFO:
+ vlan_filter_drop_vids(vlan_info, htons(ETH_P_8021Q));
+ break;
+
+ case NETDEV_SVLAN_FILTER_PUSH_INFO:
+ err = vlan_filter_push_vids(vlan_info, htons(ETH_P_8021AD));
+ if (err)
+ return notifier_from_errno(err);
+ break;
+
+ case NETDEV_SVLAN_FILTER_DROP_INFO:
+ vlan_filter_drop_vids(vlan_info, htons(ETH_P_8021AD));
+ break;
}
out:
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index a8ba510..e23aac3 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -97,6 +97,9 @@ static inline struct net_device *vlan_find_dev(struct net_device *real_dev,
if (((dev) = __vlan_group_get_device((grp), (i) / VLAN_N_VID, \
(i) % VLAN_N_VID)))
+int vlan_filter_push_vids(struct vlan_info *vlan_info, __be16 proto);
+void vlan_filter_drop_vids(struct vlan_info *vlan_info, __be16 proto);
+
/* found in vlan_dev.c */
void vlan_dev_set_ingress_priority(const struct net_device *dev,
u32 skb_prio, u16 vlan_prio);
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 45c9bf5..c8d7abd 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -165,13 +165,12 @@ struct vlan_vid_info {
int refcount;
};
-static bool vlan_hw_filter_capable(const struct net_device *dev,
- const struct vlan_vid_info *vid_info)
+bool vlan_hw_filter_capable(const struct net_device *dev, __be16 proto)
{
- if (vid_info->proto == htons(ETH_P_8021Q) &&
+ if (proto == htons(ETH_P_8021Q) &&
dev->features & NETIF_F_HW_VLAN_CTAG_FILTER)
return true;
- if (vid_info->proto == htons(ETH_P_8021AD) &&
+ if (proto == htons(ETH_P_8021AD) &&
dev->features & NETIF_F_HW_VLAN_STAG_FILTER)
return true;
return false;
@@ -202,11 +201,73 @@ static struct vlan_vid_info *vlan_vid_info_alloc(__be16 proto, u16 vid)
return vid_info;
}
+static int vlan_add_rx_filter_info(struct net_device *dev, __be16 proto, u16 vid)
+{
+ if (!vlan_hw_filter_capable(dev, proto))
+ return 0;
+
+ if (netif_device_present(dev))
+ return dev->netdev_ops->ndo_vlan_rx_add_vid(dev, proto, vid);
+ else
+ return -ENODEV;
+}
+
+static int vlan_kill_rx_filter_info(struct net_device *dev, __be16 proto, u16 vid)
+{
+ if (!vlan_hw_filter_capable(dev, proto))
+ return 0;
+
+ if (netif_device_present(dev))
+ return dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, proto, vid);
+ else
+ return -ENODEV;
+}
+
+int vlan_filter_push_vids(struct vlan_info *vlan_info, __be16 proto)
+{
+ struct net_device *real_dev = vlan_info->real_dev;
+ struct vlan_vid_info *vlan_vid_info;
+ int err;
+
+ list_for_each_entry(vlan_vid_info, &vlan_info->vid_list, list) {
+ if (vlan_vid_info->proto == proto) {
+ err = vlan_add_rx_filter_info(real_dev, proto,
+ vlan_vid_info->vid);
+ if (err)
+ goto unwind;
+ }
+ }
+
+ return 0;
+
+unwind:
+ list_for_each_entry_continue_reverse(vlan_vid_info,
+ &vlan_info->vid_list, list) {
+ if (vlan_vid_info->proto == proto)
+ vlan_kill_rx_filter_info(real_dev, proto,
+ vlan_vid_info->vid);
+ }
+
+ return err;
+}
+EXPORT_SYMBOL(vlan_filter_push_vids);
+
+void vlan_filter_drop_vids(struct vlan_info *vlan_info, __be16 proto)
+{
+ struct vlan_vid_info *vlan_vid_info;
+
+ list_for_each_entry(vlan_vid_info, &vlan_info->vid_list, list)
+ if (vlan_vid_info->proto == proto)
+ vlan_kill_rx_filter_info(vlan_info->real_dev,
+ vlan_vid_info->proto,
+ vlan_vid_info->vid);
+}
+EXPORT_SYMBOL(vlan_filter_drop_vids);
+
static int __vlan_vid_add(struct vlan_info *vlan_info, __be16 proto, u16 vid,
struct vlan_vid_info **pvid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
struct vlan_vid_info *vid_info;
int err;
@@ -214,16 +275,12 @@ static int __vlan_vid_add(struct vlan_info *vlan_info, __be16 proto, u16 vid,
if (!vid_info)
return -ENOMEM;
- if (vlan_hw_filter_capable(dev, vid_info)) {
- if (netif_device_present(dev))
- err = ops->ndo_vlan_rx_add_vid(dev, proto, vid);
- else
- err = -ENODEV;
- if (err) {
- kfree(vid_info);
- return err;
- }
+ err = vlan_add_rx_filter_info(dev, proto, vid);
+ if (err) {
+ kfree(vid_info);
+ return err;
}
+
list_add(&vid_info->list, &vlan_info->vid_list);
vlan_info->nr_vids++;
*pvid_info = vid_info;
@@ -270,21 +327,15 @@ static void __vlan_vid_del(struct vlan_info *vlan_info,
struct vlan_vid_info *vid_info)
{
struct net_device *dev = vlan_info->real_dev;
- const struct net_device_ops *ops = dev->netdev_ops;
__be16 proto = vid_info->proto;
u16 vid = vid_info->vid;
int err;
- if (vlan_hw_filter_capable(dev, vid_info)) {
- if (netif_device_present(dev))
- err = ops->ndo_vlan_rx_kill_vid(dev, proto, vid);
- else
- err = -ENODEV;
- if (err) {
- pr_warn("failed to kill vid %04x/%d for device %s\n",
- proto, vid, dev->name);
- }
- }
+ err = vlan_kill_rx_filter_info(dev, proto, vid);
+ if (err)
+ pr_warn("failed to kill vid %04x/%d for device %s\n",
+ proto, vid, dev->name);
+
list_del(&vid_info->list);
kfree(vid_info);
vlan_info->nr_vids--;
diff --git a/net/core/dev.c b/net/core/dev.c
index 97a96df..5793775 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1584,6 +1584,8 @@ const char *netdev_cmd_to_name(enum netdev_cmd cmd)
N(RESEND_IGMP) N(PRECHANGEMTU) N(CHANGEINFODATA) N(BONDING_INFO)
N(PRECHANGEUPPER) N(CHANGELOWERSTATE) N(UDP_TUNNEL_PUSH_INFO)
N(UDP_TUNNEL_DROP_INFO) N(CHANGE_TX_QUEUE_LEN)
+ N(CVLAN_FILTER_PUSH_INFO) N(CVLAN_FILTER_DROP_INFO)
+ N(SVLAN_FILTER_PUSH_INFO) N(SVLAN_FILTER_DROP_INFO)
};
#undef N
return "UNKNOWN_NETDEV_EVENT";
@@ -7660,6 +7662,24 @@ int __netdev_update_features(struct net_device *dev)
}
}
+ if (diff & NETIF_F_HW_VLAN_CTAG_FILTER) {
+ if (features & NETIF_F_HW_VLAN_CTAG_FILTER) {
+ dev->features = features;
+ err |= vlan_get_rx_ctag_filter_info(dev);
+ } else {
+ vlan_drop_rx_ctag_filter_info(dev);
+ }
+ }
+
+ if (diff & NETIF_F_HW_VLAN_STAG_FILTER) {
+ if (features & NETIF_F_HW_VLAN_STAG_FILTER) {
+ dev->features = features;
+ err |= vlan_get_rx_stag_filter_info(dev);
+ } else {
+ vlan_drop_rx_stag_filter_info(dev);
+ }
+ }
+
dev->features = features;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net v2] udp6: set dst cache for a connected sk before udp_v6_send_skb
From: Alexey Kodanev @ 2018-03-28 14:57 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, Eric Dumazet, David Miller
In-Reply-To: <20180327190002.bbrfik2pucgwepyu@kafai-mbp.dhcp.thefacebook.com>
On 27.03.2018 22:00, Martin KaFai Lau wrote:
> On Tue, Mar 27, 2018 at 04:27:30PM +0300, Alexey Kodanev wrote:
>> On 26.03.2018 20:02, Martin KaFai Lau wrote:
>>> On Mon, Mar 26, 2018 at 05:48:47PM +0300, Alexey Kodanev wrote:
>>>> After commit 33c162a980fe ("ipv6: datagram: Update dst cache of a
>>>> connected datagram sk during pmtu update"), when the error occurs on
>>>> sending datagram in udpv6_sendmsg() due to ICMPV6_PKT_TOOBIG type,
>>>> error handler can trigger the following path and call ip6_dst_store():
>>>>
>>>> udpv6_err()
>>>> ip6_sk_update_pmtu()
>>>> ip6_datagram_dst_update()
>>>> ip6_dst_lookup_flow(), can create a RTF_CACHE clone
>>> Instead of ip6_dst_lookup_flow(),
>>> you meant the RTF_CACHE route created in ip6_update_pmtu()
>>>
>>
>> Right, or even earlier... I was using vti tunnel and it invokes
>> skb_dst_update_pmtu() on this error, then sends ICMPv6_PKT_TOOBIG.
>>
>>>> ...
>>>> ip6_dst_store()
>>>>
>>>> It can happen before a connected UDP socket invokes ip6_dst_store()
>>>> in the end of udpv6_sendmsg(), on destination release, as a result,
>>>> the last one changes dst to the old one, preventing getting updated
>>>> dst cache on the next udpv6_sendmsg() call.
>>>>
>>>> This patch moves ip6_dst_store() in udpv6_sendmsg(), so that it is
>>>> invoked after ip6_sk_dst_lookup_flow() and before udp_v6_send_skb().
>>> After this patch, the above udpv6_err() path could not happen after
>>> ip6_sk_dst_lookup_flow() and before the ip6_dst_store() in udpv6_sendmsg()?
>>>
>>
>> May be we could minimize this if save it in ip6_sk_dst_lookup_flow()
>> for a connected UDP sockets only if we're not getting it from a cache
>> for some reason?
> I am just not clear how moving it earlier can completely stop the
> described issue. Beside, the next ICMPV6_PKT_TOOBIG will be
> received again and eventually rectify sk->sk_dst_cache?
With a request/response scenario in my environment, it always happens
after udp_v6_send_skb() and before the 'release_dst:' in udpv6_sendmsg().
So the socket there never gets updated cache when resending datagram
again after ICMPV6_PKT_TOOBIG... dst cache is always reset to the old
one in the end of udpv6_sendmsg().
>
>>
>> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>> index a8a9195..0204f52 100644
>> --- a/net/ipv6/ip6_output.c
>> +++ b/net/ipv6/ip6_output.c
>> @@ -1115,13 +1115,30 @@ struct dst_entry *ip6_dst_lookup_flow(const struct sock *sk, struct flowi6 *fl6,
>> * error code.
>> */
>> struct dst_entry *ip6_sk_dst_lookup_flow(struct sock *sk, struct flowi6 *fl6,
>> - const struct in6_addr *final_dst)
>> + const struct in6_addr *final_dst,
>> + bool connected)
>> {
>> struct dst_entry *dst = sk_dst_check(sk, inet6_sk(sk)->dst_cookie);
>>
>> dst = ip6_sk_dst_check(sk, dst, fl6);
>> - if (!dst)
>> - dst = ip6_dst_lookup_flow(sk, fl6, final_dst);
>> + if (dst)
>> + return dst;
>> +
>> + dst = ip6_dst_lookup_flow(sk, fl6, final_dst);
>> +
>> + if (connected && !IS_ERR(dst))
>> + ip6_dst_store(sk, dst_clone(dst), ...);
> Right, I think doing dst_store() only if ip6_sk_dst_check()/
> sk_dst_check() returns NULL is a more sensible thing to
> do here.
OK, will prepare the patch.
Thanks,
Alexey
^ permalink raw reply
* [net-next 0/4] tipc: slim down name table
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
We clean up and improve the name binding table:
- Replace the memory consuming 'sub_sequence/service range' array with
an RB tree.
- Introduce support for overlapping service sequences/ranges
Jon Maloy (4):
tipc: replace name table service range array with rb tree
tipc: refactor name table translate function
tipc: permit overlapping service ranges in name table
tipc: tipc: rename address types in user api
include/uapi/linux/tipc.h | 32 +-
net/tipc/core.h | 1 +
net/tipc/link.c | 2 +-
net/tipc/name_distr.c | 90 +---
net/tipc/name_distr.h | 1 -
net/tipc/name_table.c | 1075 ++++++++++++++++++++-------------------------
net/tipc/name_table.h | 10 +-
net/tipc/net.c | 2 +-
net/tipc/node.c | 4 +-
net/tipc/socket.c | 4 +-
net/tipc/subscr.h | 4 +-
11 files changed, 538 insertions(+), 687 deletions(-)
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply
* [net-next 1/4] tipc: replace name table service range array with rb tree
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The current design of the binding table has an unnecessary memory
consuming and complex data structure. It aggregates the service range
items into an array, which is expanded by a factor two every time it
becomes too small to hold a new item. Furthermore, the arrays never
shrink when the number of ranges diminishes.
We now replace this array with an RB tree that is holding the range
items as tree nodes, each range directly holding a list of bindings.
This, along with a few name changes, improves both readability and
volume of the code, as well as reducing memory consumption and hopefully
improving cache hit rate.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/core.h | 1 +
net/tipc/link.c | 2 +-
net/tipc/name_table.c | 1032 ++++++++++++++++++++++---------------------------
net/tipc/name_table.h | 2 +-
net/tipc/node.c | 4 +-
net/tipc/subscr.h | 4 +-
6 files changed, 477 insertions(+), 568 deletions(-)
diff --git a/net/tipc/core.h b/net/tipc/core.h
index d0f64ca..8020a6c 100644
--- a/net/tipc/core.h
+++ b/net/tipc/core.h
@@ -58,6 +58,7 @@
#include <linux/etherdevice.h>
#include <net/netns/generic.h>
#include <linux/rhashtable.h>
+#include <net/genetlink.h>
struct tipc_node;
struct tipc_bearer;
diff --git a/net/tipc/link.c b/net/tipc/link.c
index 1289b4b..8f2a949 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1810,7 +1810,7 @@ int tipc_link_bc_nack_rcv(struct tipc_link *l, struct sk_buff *skb,
void tipc_link_set_queue_limits(struct tipc_link *l, u32 win)
{
- int max_bulk = TIPC_MAX_PUBLICATIONS / (l->mtu / ITEM_SIZE);
+ int max_bulk = TIPC_MAX_PUBL / (l->mtu / ITEM_SIZE);
l->window = win;
l->backlog[TIPC_LOW_IMPORTANCE].limit = max_t(u16, 50, win);
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 4359605..c309402 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -44,52 +44,40 @@
#include "addr.h"
#include "node.h"
#include "group.h"
-#include <net/genetlink.h>
-
-#define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
/**
- * struct name_info - name sequence publication info
- * @node_list: list of publications on own node of this <type,lower,upper>
- * @all_publ: list of all publications of this <type,lower,upper>
+ * struct service_range - container for all bindings of a service range
+ * @lower: service range lower bound
+ * @upper: service range upper bound
+ * @tree_node: member of service range RB tree
+ * @local_publ: list of identical publications made from this node
+ * Used by closest_first lookup and multicast lookup algorithm
+ * @all_publ: all publications identical to this one, whatever node and scope
+ * Used by round-robin lookup algorithm
*/
-struct name_info {
- struct list_head local_publ;
- struct list_head all_publ;
-};
-
-/**
- * struct sub_seq - container for all published instances of a name sequence
- * @lower: name sequence lower bound
- * @upper: name sequence upper bound
- * @info: pointer to name sequence publication info
- */
-struct sub_seq {
+struct service_range {
u32 lower;
u32 upper;
- struct name_info *info;
+ struct rb_node tree_node;
+ struct list_head local_publ;
+ struct list_head all_publ;
};
/**
- * struct name_seq - container for all published instances of a name type
- * @type: 32 bit 'type' value for name sequence
- * @sseq: pointer to dynamically-sized array of sub-sequences of this 'type';
- * sub-sequences are sorted in ascending order
- * @alloc: number of sub-sequences currently in array
- * @first_free: array index of first unused sub-sequence entry
- * @ns_list: links to adjacent name sequences in hash chain
- * @subscriptions: list of subscriptions for this 'type'
- * @lock: spinlock controlling access to publication lists of all sub-sequences
+ * struct tipc_service - container for all published instances of a service type
+ * @type: 32 bit 'type' value for service
+ * @ranges: rb tree containing all service ranges for this service
+ * @service_list: links to adjacent name ranges in hash chain
+ * @subscriptions: list of subscriptions for this service type
+ * @lock: spinlock controlling access to pertaining service ranges/publications
* @rcu: RCU callback head used for deferred freeing
*/
-struct name_seq {
+struct tipc_service {
u32 type;
- struct sub_seq *sseqs;
- u32 alloc;
- u32 first_free;
- struct hlist_node ns_list;
+ struct rb_root ranges;
+ struct hlist_node service_list;
struct list_head subscriptions;
- spinlock_t lock;
+ spinlock_t lock; /* Covers service range list */
struct rcu_head rcu;
};
@@ -99,17 +87,16 @@ static int hash(int x)
}
/**
- * publ_create - create a publication structure
+ * tipc_publ_create - create a publication structure
*/
-static struct publication *publ_create(u32 type, u32 lower, u32 upper,
- u32 scope, u32 node, u32 port,
- u32 key)
+static struct publication *tipc_publ_create(u32 type, u32 lower, u32 upper,
+ u32 scope, u32 node, u32 port,
+ u32 key)
{
struct publication *publ = kzalloc(sizeof(*publ), GFP_ATOMIC);
- if (publ == NULL) {
- pr_warn("Publication creation failure, no memory\n");
+
+ if (!publ)
return NULL;
- }
publ->type = type;
publ->lower = lower;
@@ -119,372 +106,298 @@ static struct publication *publ_create(u32 type, u32 lower, u32 upper,
publ->port = port;
publ->key = key;
INIT_LIST_HEAD(&publ->binding_sock);
+ INIT_LIST_HEAD(&publ->binding_node);
+ INIT_LIST_HEAD(&publ->local_publ);
+ INIT_LIST_HEAD(&publ->all_publ);
return publ;
}
/**
- * tipc_subseq_alloc - allocate a specified number of sub-sequence structures
- */
-static struct sub_seq *tipc_subseq_alloc(u32 cnt)
-{
- return kcalloc(cnt, sizeof(struct sub_seq), GFP_ATOMIC);
-}
-
-/**
- * tipc_nameseq_create - create a name sequence structure for the specified 'type'
+ * tipc_service_create - create a service structure for the specified 'type'
*
- * Allocates a single sub-sequence structure and sets it to all 0's.
+ * Allocates a single range structure and sets it to all 0's.
*/
-static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_head)
+static struct tipc_service *tipc_service_create(u32 type, struct hlist_head *hd)
{
- struct name_seq *nseq = kzalloc(sizeof(*nseq), GFP_ATOMIC);
- struct sub_seq *sseq = tipc_subseq_alloc(1);
+ struct tipc_service *service = kzalloc(sizeof(*service), GFP_ATOMIC);
- if (!nseq || !sseq) {
- pr_warn("Name sequence creation failed, no memory\n");
- kfree(nseq);
- kfree(sseq);
+ if (!service) {
+ pr_warn("Service creation failed, no memory\n");
return NULL;
}
- spin_lock_init(&nseq->lock);
- nseq->type = type;
- nseq->sseqs = sseq;
- nseq->alloc = 1;
- INIT_HLIST_NODE(&nseq->ns_list);
- INIT_LIST_HEAD(&nseq->subscriptions);
- hlist_add_head_rcu(&nseq->ns_list, seq_head);
- return nseq;
+ spin_lock_init(&service->lock);
+ service->type = type;
+ service->ranges = RB_ROOT;
+ INIT_HLIST_NODE(&service->service_list);
+ INIT_LIST_HEAD(&service->subscriptions);
+ hlist_add_head_rcu(&service->service_list, hd);
+ return service;
}
/**
- * nameseq_find_subseq - find sub-sequence (if any) matching a name instance
+ * tipc_service_find_range - find service range matching a service instance
*
- * Very time-critical, so binary searches through sub-sequence array.
+ * Very time-critical, so binary search through range rb tree
*/
-static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
- u32 instance)
+static struct service_range *tipc_service_find_range(struct tipc_service *sc,
+ u32 instance)
{
- struct sub_seq *sseqs = nseq->sseqs;
- int low = 0;
- int high = nseq->first_free - 1;
- int mid;
-
- while (low <= high) {
- mid = (low + high) / 2;
- if (instance < sseqs[mid].lower)
- high = mid - 1;
- else if (instance > sseqs[mid].upper)
- low = mid + 1;
+ struct rb_node *n = sc->ranges.rb_node;
+ struct service_range *sr;
+
+ while (n) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower > instance)
+ n = n->rb_left;
+ else if (sr->upper < instance)
+ n = n->rb_right;
else
- return &sseqs[mid];
+ return sr;
}
return NULL;
}
-/**
- * nameseq_locate_subseq - determine position of name instance in sub-sequence
- *
- * Returns index in sub-sequence array of the entry that contains the specified
- * instance value; if no entry contains that value, returns the position
- * where a new entry for it would be inserted in the array.
- *
- * Note: Similar to binary search code for locating a sub-sequence.
- */
-static u32 nameseq_locate_subseq(struct name_seq *nseq, u32 instance)
+static struct service_range *tipc_service_create_range(struct tipc_service *sc,
+ u32 lower, u32 upper)
{
- struct sub_seq *sseqs = nseq->sseqs;
- int low = 0;
- int high = nseq->first_free - 1;
- int mid;
-
- while (low <= high) {
- mid = (low + high) / 2;
- if (instance < sseqs[mid].lower)
- high = mid - 1;
- else if (instance > sseqs[mid].upper)
- low = mid + 1;
+ struct rb_node **n, *parent = NULL;
+ struct service_range *sr, *tmp;
+
+ n = &sc->ranges.rb_node;
+ while (*n) {
+ tmp = container_of(*n, struct service_range, tree_node);
+ parent = *n;
+ tmp = container_of(parent, struct service_range, tree_node);
+ if (lower < tmp->lower)
+ n = &(*n)->rb_left;
+ else if (upper > tmp->upper)
+ n = &(*n)->rb_right;
else
- return mid;
+ return NULL;
}
- return low;
+ sr = kzalloc(sizeof(*sr), GFP_ATOMIC);
+ if (!sr)
+ return NULL;
+ sr->lower = lower;
+ sr->upper = upper;
+ INIT_LIST_HEAD(&sr->local_publ);
+ INIT_LIST_HEAD(&sr->all_publ);
+ rb_link_node(&sr->tree_node, parent, n);
+ rb_insert_color(&sr->tree_node, &sc->ranges);
+ return sr;
}
-/**
- * tipc_nameseq_insert_publ
- */
-static struct publication *tipc_nameseq_insert_publ(struct net *net,
- struct name_seq *nseq,
+static struct publication *tipc_service_insert_publ(struct net *net,
+ struct tipc_service *sc,
u32 type, u32 lower,
u32 upper, u32 scope,
- u32 node, u32 port, u32 key)
+ u32 node, u32 port,
+ u32 key)
{
- struct tipc_subscription *s;
- struct tipc_subscription *st;
- struct publication *publ;
- struct sub_seq *sseq;
- struct name_info *info;
- int created_subseq = 0;
-
- sseq = nameseq_find_subseq(nseq, lower);
- if (sseq) {
-
- /* Lower end overlaps existing entry => need an exact match */
- if ((sseq->lower != lower) || (sseq->upper != upper)) {
- return NULL;
- }
-
- info = sseq->info;
-
- /* Check if an identical publication already exists */
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->port == port && publ->key == key &&
- (!publ->node || publ->node == node))
- return NULL;
- }
- } else {
- u32 inspos;
- struct sub_seq *freesseq;
-
- /* Find where lower end should be inserted */
- inspos = nameseq_locate_subseq(nseq, lower);
-
- /* Fail if upper end overlaps into an existing entry */
- if ((inspos < nseq->first_free) &&
- (upper >= nseq->sseqs[inspos].lower)) {
- return NULL;
- }
-
- /* Ensure there is space for new sub-sequence */
- if (nseq->first_free == nseq->alloc) {
- struct sub_seq *sseqs = tipc_subseq_alloc(nseq->alloc * 2);
-
- if (!sseqs) {
- pr_warn("Cannot publish {%u,%u,%u}, no memory\n",
- type, lower, upper);
- return NULL;
- }
- memcpy(sseqs, nseq->sseqs,
- nseq->alloc * sizeof(struct sub_seq));
- kfree(nseq->sseqs);
- nseq->sseqs = sseqs;
- nseq->alloc *= 2;
- }
-
- info = kzalloc(sizeof(*info), GFP_ATOMIC);
- if (!info) {
- pr_warn("Cannot publish {%u,%u,%u}, no memory\n",
- type, lower, upper);
- return NULL;
- }
-
- INIT_LIST_HEAD(&info->local_publ);
- INIT_LIST_HEAD(&info->all_publ);
-
- /* Insert new sub-sequence */
- sseq = &nseq->sseqs[inspos];
- freesseq = &nseq->sseqs[nseq->first_free];
- memmove(sseq + 1, sseq, (freesseq - sseq) * sizeof(*sseq));
- memset(sseq, 0, sizeof(*sseq));
- nseq->first_free++;
- sseq->lower = lower;
- sseq->upper = upper;
- sseq->info = info;
- created_subseq = 1;
+ struct tipc_subscription *sub, *tmp;
+ struct service_range *sr;
+ struct publication *p;
+ bool first = false;
+
+ sr = tipc_service_find_range(sc, lower);
+ if (!sr) {
+ sr = tipc_service_create_range(sc, lower, upper);
+ if (!sr)
+ goto err;
+ first = true;
}
- /* Insert a publication */
- publ = publ_create(type, lower, upper, scope, node, port, key);
- if (!publ)
+ /* Lower end overlaps existing entry, but we need an exact match */
+ if (sr->lower != lower || sr->upper != upper)
return NULL;
- list_add(&publ->all_publ, &info->all_publ);
+ /* Return if the publication already exists */
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->key == key && (!p->node || p->node == node))
+ return NULL;
+ }
+ /* Create and insert publication */
+ p = tipc_publ_create(type, lower, upper, scope, node, port, key);
+ if (!p)
+ goto err;
if (in_own_node(net, node))
- list_add(&publ->local_publ, &info->local_publ);
+ list_add(&p->local_publ, &sr->local_publ);
+ list_add(&p->all_publ, &sr->all_publ);
/* Any subscriptions waiting for notification? */
- list_for_each_entry_safe(s, st, &nseq->subscriptions, nameseq_list) {
- tipc_sub_report_overlap(s, publ->lower, publ->upper,
- TIPC_PUBLISHED, publ->port,
- publ->node, publ->scope,
- created_subseq);
+ list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
+ tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_PUBLISHED,
+ p->port, p->node, p->scope, first);
}
- return publ;
+ return p;
+err:
+ pr_warn("Failed to bind to %u,%u,%u, no memory\n", type, lower, upper);
+ return NULL;
}
/**
- * tipc_nameseq_remove_publ
+ * tipc_service_remove_publ - remove a publication from a service
*
* NOTE: There may be cases where TIPC is asked to remove a publication
* that is not in the name table. For example, if another node issues a
- * publication for a name sequence that overlaps an existing name sequence
+ * publication for a name range that overlaps an existing name range
* the publication will not be recorded, which means the publication won't
- * be found when the name sequence is later withdrawn by that node.
+ * be found when the name range is later withdrawn by that node.
* A failed withdraw request simply returns a failure indication and lets the
* caller issue any error or warning messages associated with such a problem.
*/
-static struct publication *tipc_nameseq_remove_publ(struct net *net,
- struct name_seq *nseq,
+static struct publication *tipc_service_remove_publ(struct net *net,
+ struct tipc_service *sc,
u32 inst, u32 node,
u32 port, u32 key)
{
- struct publication *publ;
- struct sub_seq *sseq = nameseq_find_subseq(nseq, inst);
- struct name_info *info;
- struct sub_seq *free;
- struct tipc_subscription *s, *st;
- int removed_subseq = 0;
-
- if (!sseq)
- return NULL;
+ struct tipc_subscription *sub, *tmp;
+ struct service_range *sr;
+ struct publication *p;
+ bool found = false;
+ bool last = false;
- info = sseq->info;
+ sr = tipc_service_find_range(sc, inst);
+ if (!sr)
+ return NULL;
- /* Locate publication, if it exists */
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->key == key && publ->port == port &&
- (!publ->node || publ->node == node))
- goto found;
+ /* Find publication, if it exists */
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->key != key || (node && node != p->node))
+ continue;
+ found = true;
+ break;
}
- return NULL;
+ if (!found)
+ return NULL;
-found:
- list_del(&publ->all_publ);
- if (in_own_node(net, node))
- list_del(&publ->local_publ);
-
- /* Contract subseq list if no more publications for that subseq */
- if (list_empty(&info->all_publ)) {
- kfree(info);
- free = &nseq->sseqs[nseq->first_free--];
- memmove(sseq, sseq + 1, (free - (sseq + 1)) * sizeof(*sseq));
- removed_subseq = 1;
+ list_del(&p->all_publ);
+ list_del(&p->local_publ);
+
+ /* Remove service range item if this was its last publication */
+ if (list_empty(&sr->all_publ)) {
+ last = true;
+ rb_erase(&sr->tree_node, &sc->ranges);
+ kfree(sr);
}
/* Notify any waiting subscriptions */
- list_for_each_entry_safe(s, st, &nseq->subscriptions, nameseq_list) {
- tipc_sub_report_overlap(s, publ->lower, publ->upper,
- TIPC_WITHDRAWN, publ->port,
- publ->node, publ->scope,
- removed_subseq);
+ list_for_each_entry_safe(sub, tmp, &sc->subscriptions, service_list) {
+ tipc_sub_report_overlap(sub, p->lower, p->upper, TIPC_WITHDRAWN,
+ p->port, p->node, p->scope, last);
}
-
- return publ;
+ return p;
}
/**
- * tipc_nameseq_subscribe - attach a subscription, and optionally
- * issue the prescribed number of events if there is any sub-
- * sequence overlapping with the requested sequence
+ * tipc_service_subscribe - attach a subscription, and optionally
+ * issue the prescribed number of events if there is any service
+ * range overlapping with the requested range
*/
-static void tipc_nameseq_subscribe(struct name_seq *nseq,
+static void tipc_service_subscribe(struct tipc_service *service,
struct tipc_subscription *sub)
{
- struct sub_seq *sseq = nseq->sseqs;
+ struct tipc_subscr *sb = &sub->evt.s;
+ struct service_range *sr;
struct tipc_name_seq ns;
- struct tipc_subscr *s = &sub->evt.s;
- bool no_status;
+ struct publication *p;
+ struct rb_node *n;
+ bool first;
- ns.type = tipc_sub_read(s, seq.type);
- ns.lower = tipc_sub_read(s, seq.lower);
- ns.upper = tipc_sub_read(s, seq.upper);
- no_status = tipc_sub_read(s, filter) & TIPC_SUB_NO_STATUS;
+ ns.type = tipc_sub_read(sb, seq.type);
+ ns.lower = tipc_sub_read(sb, seq.lower);
+ ns.upper = tipc_sub_read(sb, seq.upper);
tipc_sub_get(sub);
- list_add(&sub->nameseq_list, &nseq->subscriptions);
+ list_add(&sub->service_list, &service->subscriptions);
- if (no_status || !sseq)
+ if (tipc_sub_read(sb, filter) & TIPC_SUB_NO_STATUS)
return;
- while (sseq != &nseq->sseqs[nseq->first_free]) {
- if (tipc_sub_check_overlap(&ns, sseq->lower, sseq->upper)) {
- struct publication *crs;
- struct name_info *info = sseq->info;
- int must_report = 1;
-
- list_for_each_entry(crs, &info->all_publ, all_publ) {
- tipc_sub_report_overlap(sub, sseq->lower,
- sseq->upper,
- TIPC_PUBLISHED,
- crs->port,
- crs->node,
- crs->scope,
- must_report);
- must_report = 0;
- }
+ for (n = rb_first(&service->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower > ns.upper)
+ break;
+ if (!tipc_sub_check_overlap(&ns, sr->lower, sr->upper))
+ continue;
+ first = true;
+
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ tipc_sub_report_overlap(sub, sr->lower, sr->upper,
+ TIPC_PUBLISHED, p->port,
+ p->node, p->scope, first);
+ first = false;
}
- sseq++;
}
}
-static struct name_seq *nametbl_find_seq(struct net *net, u32 type)
+static struct tipc_service *tipc_service_find(struct net *net, u32 type)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct hlist_head *seq_head;
- struct name_seq *ns;
-
- seq_head = &tn->nametbl->seq_hlist[hash(type)];
- hlist_for_each_entry_rcu(ns, seq_head, ns_list) {
- if (ns->type == type)
- return ns;
+ struct name_table *nt = tipc_name_table(net);
+ struct hlist_head *service_head;
+ struct tipc_service *service;
+
+ service_head = &nt->services[hash(type)];
+ hlist_for_each_entry_rcu(service, service_head, service_list) {
+ if (service->type == type)
+ return service;
}
-
return NULL;
};
struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
- u32 lower, u32 upper, u32 scope,
- u32 node, u32 port, u32 key)
+ u32 lower, u32 upper,
+ u32 scope, u32 node,
+ u32 port, u32 key)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct publication *publ;
- struct name_seq *seq = nametbl_find_seq(net, type);
- int index = hash(type);
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_service *sc;
+ struct publication *p;
if (scope > TIPC_NODE_SCOPE || lower > upper) {
- pr_debug("Failed to publish illegal {%u,%u,%u} with scope %u\n",
+ pr_debug("Failed to bind illegal {%u,%u,%u} with scope %u\n",
type, lower, upper, scope);
return NULL;
}
-
- if (!seq)
- seq = tipc_nameseq_create(type, &tn->nametbl->seq_hlist[index]);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
+ sc = tipc_service_create(type, &nt->services[hash(type)]);
+ if (!sc)
return NULL;
- spin_lock_bh(&seq->lock);
- publ = tipc_nameseq_insert_publ(net, seq, type, lower, upper,
- scope, node, port, key);
- spin_unlock_bh(&seq->lock);
- return publ;
+ spin_lock_bh(&sc->lock);
+ p = tipc_service_insert_publ(net, sc, type, lower, upper,
+ scope, node, port, key);
+ spin_unlock_bh(&sc->lock);
+ return p;
}
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 port,
- u32 key)
+ u32 lower, u32 node, u32 port,
+ u32 key)
{
- struct publication *publ;
- struct name_seq *seq = nametbl_find_seq(net, type);
+ struct tipc_service *sc = tipc_service_find(net, type);
+ struct publication *p = NULL;
- if (!seq)
+ if (!sc)
return NULL;
- spin_lock_bh(&seq->lock);
- publ = tipc_nameseq_remove_publ(net, seq, lower, node, port, key);
- if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
- kfree_rcu(seq, rcu);
- return publ;
+ spin_lock_bh(&sc->lock);
+ p = tipc_service_remove_publ(net, sc, lower, node, port, key);
+
+ /* Delete service item if this no more publications and subscriptions */
+ if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
+ hlist_del_init_rcu(&sc->service_list);
+ kfree_rcu(sc, rcu);
}
- spin_unlock_bh(&seq->lock);
- return publ;
+ spin_unlock_bh(&sc->lock);
+ return p;
}
/**
- * tipc_nametbl_translate - perform name translation
+ * tipc_nametbl_translate - perform service instance to socket translation
*
* On entry, 'destnode' is the search domain used during translation.
*
@@ -492,7 +405,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
* - if name translation is deferred to another node/cluster/zone,
* leaves 'destnode' unchanged (will be non-zero) and returns 0
* - if name translation is attempted and succeeds, sets 'destnode'
- * to publishing node and returns port reference (will be non-zero)
+ * to publication node and returns port reference (will be non-zero)
* - if name translation is attempted and fails, sets 'destnode' to 0
* and returns 0
*/
@@ -502,10 +415,9 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
struct tipc_net *tn = tipc_net(net);
bool legacy = tn->legacy_addr_format;
u32 self = tipc_own_addr(net);
- struct sub_seq *sseq;
- struct name_info *info;
- struct publication *publ;
- struct name_seq *seq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
u32 port = 0;
u32 node = 0;
@@ -513,49 +425,49 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
return 0;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (unlikely(!seq))
+ sc = tipc_service_find(net, type);
+ if (unlikely(!sc))
goto not_found;
- spin_lock_bh(&seq->lock);
- sseq = nameseq_find_subseq(seq, instance);
- if (unlikely(!sseq))
+
+ spin_lock_bh(&sc->lock);
+ sr = tipc_service_find_range(sc, instance);
+ if (unlikely(!sr))
goto no_match;
- info = sseq->info;
/* Closest-First Algorithm */
if (legacy && !*destnode) {
- if (!list_empty(&info->local_publ)) {
- publ = list_first_entry(&info->local_publ,
- struct publication,
- local_publ);
- list_move_tail(&publ->local_publ,
- &info->local_publ);
+ if (!list_empty(&sr->local_publ)) {
+ p = list_first_entry(&sr->local_publ,
+ struct publication,
+ local_publ);
+ list_move_tail(&p->local_publ,
+ &sr->local_publ);
} else {
- publ = list_first_entry(&info->all_publ,
- struct publication,
- all_publ);
- list_move_tail(&publ->all_publ,
- &info->all_publ);
+ p = list_first_entry(&sr->all_publ,
+ struct publication,
+ all_publ);
+ list_move_tail(&p->all_publ,
+ &sr->all_publ);
}
}
/* Round-Robin Algorithm */
- else if (*destnode == tipc_own_addr(net)) {
- if (list_empty(&info->local_publ))
+ else if (*destnode == self) {
+ if (list_empty(&sr->local_publ))
goto no_match;
- publ = list_first_entry(&info->local_publ, struct publication,
- local_publ);
- list_move_tail(&publ->local_publ, &info->local_publ);
+ p = list_first_entry(&sr->local_publ, struct publication,
+ local_publ);
+ list_move_tail(&p->local_publ, &sr->local_publ);
} else {
- publ = list_first_entry(&info->all_publ, struct publication,
- all_publ);
- list_move_tail(&publ->all_publ, &info->all_publ);
+ p = list_first_entry(&sr->all_publ, struct publication,
+ all_publ);
+ list_move_tail(&p->all_publ, &sr->all_publ);
}
- port = publ->port;
- node = publ->node;
+ port = p->port;
+ node = p->node;
no_match:
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
not_found:
rcu_read_unlock();
*destnode = node;
@@ -567,34 +479,36 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
bool all)
{
u32 self = tipc_own_addr(net);
- struct publication *publ;
- struct name_info *info;
- struct name_seq *seq;
- struct sub_seq *sseq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
*dstcnt = 0;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (unlikely(!seq))
+ sc = tipc_service_find(net, type);
+ if (unlikely(!sc))
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = nameseq_find_subseq(seq, instance);
- if (likely(sseq)) {
- info = sseq->info;
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- if (publ->scope != scope)
- continue;
- if (publ->port == exclude && publ->node == self)
- continue;
- tipc_dest_push(dsts, publ->node, publ->port);
- (*dstcnt)++;
- if (all)
- continue;
- list_move_tail(&publ->all_publ, &info->all_publ);
- break;
- }
+
+ spin_lock_bh(&sc->lock);
+
+ sr = tipc_service_find_range(sc, instance);
+ if (!sr)
+ goto no_match;
+
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ if (p->scope != scope)
+ continue;
+ if (p->port == exclude && p->node == self)
+ continue;
+ tipc_dest_push(dsts, p->node, p->port);
+ (*dstcnt)++;
+ if (all)
+ continue;
+ list_move_tail(&p->all_publ, &sr->all_publ);
+ break;
}
- spin_unlock_bh(&seq->lock);
+no_match:
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
return !list_empty(dsts);
@@ -603,61 +517,64 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 scope,
void tipc_nametbl_mc_lookup(struct net *net, u32 type, u32 lower, u32 upper,
u32 scope, bool exact, struct list_head *dports)
{
- struct sub_seq *sseq_stop;
- struct name_info *info;
+ struct service_range *sr;
+ struct tipc_service *sc;
struct publication *p;
- struct name_seq *seq;
- struct sub_seq *sseq;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs + nameseq_locate_subseq(seq, lower);
- sseq_stop = seq->sseqs + seq->first_free;
- for (; sseq != sseq_stop; sseq++) {
- if (sseq->lower > upper)
+ spin_lock_bh(&sc->lock);
+
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper < lower)
+ continue;
+ if (sr->lower > upper)
break;
- info = sseq->info;
- list_for_each_entry(p, &info->local_publ, local_publ) {
+ list_for_each_entry(p, &sr->local_publ, local_publ) {
if (p->scope == scope || (!exact && p->scope < scope))
tipc_dest_push(dports, 0, p->port);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
/* tipc_nametbl_lookup_dst_nodes - find broadcast destination nodes
* - Creates list of nodes that overlap the given multicast address
- * - Determines if any node local ports overlap
+ * - Determines if any node local destinations overlap
*/
void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
u32 upper, struct tipc_nlist *nodes)
{
- struct sub_seq *sseq, *stop;
- struct publication *publ;
- struct name_info *info;
- struct name_seq *seq;
+ struct service_range *sr;
+ struct tipc_service *sc;
+ struct publication *p;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs + nameseq_locate_subseq(seq, lower);
- stop = seq->sseqs + seq->first_free;
- for (; sseq != stop && sseq->lower <= upper; sseq++) {
- info = sseq->info;
- list_for_each_entry(publ, &info->all_publ, all_publ) {
- tipc_nlist_add(nodes, publ->node);
+ spin_lock_bh(&sc->lock);
+
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper < lower)
+ continue;
+ if (sr->lower > upper)
+ break;
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
+ tipc_nlist_add(nodes, p->node);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
@@ -667,89 +584,88 @@ void tipc_nametbl_lookup_dst_nodes(struct net *net, u32 type, u32 lower,
void tipc_nametbl_build_group(struct net *net, struct tipc_group *grp,
u32 type, u32 scope)
{
- struct sub_seq *sseq, *stop;
- struct name_info *info;
+ struct service_range *sr;
+ struct tipc_service *sc;
struct publication *p;
- struct name_seq *seq;
+ struct rb_node *n;
rcu_read_lock();
- seq = nametbl_find_seq(net, type);
- if (!seq)
+ sc = tipc_service_find(net, type);
+ if (!sc)
goto exit;
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs;
- stop = seq->sseqs + seq->first_free;
- for (; sseq != stop; sseq++) {
- info = sseq->info;
- list_for_each_entry(p, &info->all_publ, all_publ) {
+ spin_lock_bh(&sc->lock);
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ list_for_each_entry(p, &sr->all_publ, all_publ) {
if (p->scope != scope)
continue;
tipc_group_add_member(grp, p->node, p->port, p->lower);
}
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&sc->lock);
exit:
rcu_read_unlock();
}
-/*
- * tipc_nametbl_publish - add name publication to network name tables
+/* tipc_nametbl_publish - add service binding to name table
*/
struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
- u32 upper, u32 scope, u32 port_ref,
+ u32 upper, u32 scope, u32 port,
u32 key)
{
- struct publication *publ;
- struct sk_buff *buf = NULL;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ struct sk_buff *skb = NULL;
+ struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- if (tn->nametbl->local_publ_count >= TIPC_MAX_PUBLICATIONS) {
- pr_warn("Publication failed, local publication limit reached (%u)\n",
- TIPC_MAX_PUBLICATIONS);
- spin_unlock_bh(&tn->nametbl_lock);
- return NULL;
+
+ if (nt->local_publ_count >= TIPC_MAX_PUBL) {
+ pr_warn("Bind failed, max limit %u reached\n", TIPC_MAX_PUBL);
+ goto exit;
}
- publ = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
- tipc_own_addr(net), port_ref, key);
- if (likely(publ)) {
- tn->nametbl->local_publ_count++;
- buf = tipc_named_publish(net, publ);
+ p = tipc_nametbl_insert_publ(net, type, lower, upper, scope,
+ tipc_own_addr(net), port, key);
+ if (p) {
+ nt->local_publ_count++;
+ skb = tipc_named_publish(net, p);
/* Any pending external events? */
tipc_named_process_backlog(net);
}
+exit:
spin_unlock_bh(&tn->nametbl_lock);
- if (buf)
- tipc_node_broadcast(net, buf);
- return publ;
+ if (skb)
+ tipc_node_broadcast(net, skb);
+ return p;
}
/**
- * tipc_nametbl_withdraw - withdraw name publication from network name tables
+ * tipc_nametbl_withdraw - withdraw a service binding
*/
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 port,
- u32 key)
+int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
+ u32 port, u32 key)
{
- struct publication *publ;
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ u32 self = tipc_own_addr(net);
struct sk_buff *skb = NULL;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- publ = tipc_nametbl_remove_publ(net, type, lower, tipc_own_addr(net),
- port, key);
- if (likely(publ)) {
- tn->nametbl->local_publ_count--;
- skb = tipc_named_withdraw(net, publ);
+
+ p = tipc_nametbl_remove_publ(net, type, lower, self, port, key);
+ if (p) {
+ nt->local_publ_count--;
+ skb = tipc_named_withdraw(net, p);
/* Any pending external events? */
tipc_named_process_backlog(net);
- list_del_init(&publ->binding_sock);
- kfree_rcu(publ, rcu);
+ list_del_init(&p->binding_sock);
+ kfree_rcu(p, rcu);
} else {
- pr_err("Unable to remove local publication\n"
- "(type=%u, lower=%u, port=%u, key=%u)\n",
+ pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
type, lower, port, key);
}
spin_unlock_bh(&tn->nametbl_lock);
@@ -766,27 +682,24 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 port,
*/
void tipc_nametbl_subscribe(struct tipc_subscription *sub)
{
+ struct name_table *nt = tipc_name_table(sub->net);
struct tipc_net *tn = tipc_net(sub->net);
struct tipc_subscr *s = &sub->evt.s;
u32 type = tipc_sub_read(s, seq.type);
- int index = hash(type);
- struct name_seq *seq;
- struct tipc_name_seq ns;
+ struct tipc_service *sc;
spin_lock_bh(&tn->nametbl_lock);
- seq = nametbl_find_seq(sub->net, type);
- if (!seq)
- seq = tipc_nameseq_create(type, &tn->nametbl->seq_hlist[index]);
- if (seq) {
- spin_lock_bh(&seq->lock);
- tipc_nameseq_subscribe(seq, sub);
- spin_unlock_bh(&seq->lock);
+ sc = tipc_service_find(sub->net, type);
+ if (!sc)
+ sc = tipc_service_create(type, &nt->services[hash(type)]);
+ if (sc) {
+ spin_lock_bh(&sc->lock);
+ tipc_service_subscribe(sc, sub);
+ spin_unlock_bh(&sc->lock);
} else {
- ns.type = tipc_sub_read(s, seq.type);
- ns.lower = tipc_sub_read(s, seq.lower);
- ns.upper = tipc_sub_read(s, seq.upper);
- pr_warn("Failed to create subscription for {%u,%u,%u}\n",
- ns.type, ns.lower, ns.upper);
+ pr_warn("Failed to subscribe for {%u,%u,%u}\n", type,
+ tipc_sub_read(s, seq.lower),
+ tipc_sub_read(s, seq.upper));
}
spin_unlock_bh(&tn->nametbl_lock);
}
@@ -796,124 +709,122 @@ void tipc_nametbl_subscribe(struct tipc_subscription *sub)
*/
void tipc_nametbl_unsubscribe(struct tipc_subscription *sub)
{
- struct tipc_subscr *s = &sub->evt.s;
struct tipc_net *tn = tipc_net(sub->net);
- struct name_seq *seq;
+ struct tipc_subscr *s = &sub->evt.s;
u32 type = tipc_sub_read(s, seq.type);
+ struct tipc_service *sc;
spin_lock_bh(&tn->nametbl_lock);
- seq = nametbl_find_seq(sub->net, type);
- if (seq != NULL) {
- spin_lock_bh(&seq->lock);
- list_del_init(&sub->nameseq_list);
- tipc_sub_put(sub);
- if (!seq->first_free && list_empty(&seq->subscriptions)) {
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
- kfree_rcu(seq, rcu);
- } else {
- spin_unlock_bh(&seq->lock);
- }
+ sc = tipc_service_find(sub->net, type);
+ if (!sc)
+ goto exit;
+
+ spin_lock_bh(&sc->lock);
+ list_del_init(&sub->service_list);
+ tipc_sub_put(sub);
+
+ /* Delete service item if no more publications and subscriptions */
+ if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
+ hlist_del_init_rcu(&sc->service_list);
+ kfree_rcu(sc, rcu);
}
+ spin_unlock_bh(&sc->lock);
+exit:
spin_unlock_bh(&tn->nametbl_lock);
}
int tipc_nametbl_init(struct net *net)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct name_table *tipc_nametbl;
+ struct tipc_net *tn = tipc_net(net);
+ struct name_table *nt;
int i;
- tipc_nametbl = kzalloc(sizeof(*tipc_nametbl), GFP_ATOMIC);
- if (!tipc_nametbl)
+ nt = kzalloc(sizeof(*nt), GFP_ATOMIC);
+ if (!nt)
return -ENOMEM;
for (i = 0; i < TIPC_NAMETBL_SIZE; i++)
- INIT_HLIST_HEAD(&tipc_nametbl->seq_hlist[i]);
+ INIT_HLIST_HEAD(&nt->services[i]);
- INIT_LIST_HEAD(&tipc_nametbl->node_scope);
- INIT_LIST_HEAD(&tipc_nametbl->cluster_scope);
- tn->nametbl = tipc_nametbl;
+ INIT_LIST_HEAD(&nt->node_scope);
+ INIT_LIST_HEAD(&nt->cluster_scope);
+ tn->nametbl = nt;
spin_lock_init(&tn->nametbl_lock);
return 0;
}
/**
- * tipc_purge_publications - remove all publications for a given type
- *
- * tipc_nametbl_lock must be held when calling this function
+ * tipc_service_delete - purge all publications for a service and delete it
*/
-static void tipc_purge_publications(struct net *net, struct name_seq *seq)
+static void tipc_service_delete(struct net *net, struct tipc_service *sc)
{
- struct publication *publ, *safe;
- struct sub_seq *sseq;
- struct name_info *info;
-
- spin_lock_bh(&seq->lock);
- sseq = seq->sseqs;
- info = sseq->info;
- list_for_each_entry_safe(publ, safe, &info->all_publ, all_publ) {
- tipc_nameseq_remove_publ(net, seq, publ->lower, publ->node,
- publ->port, publ->key);
- kfree_rcu(publ, rcu);
+ struct service_range *sr, *tmpr;
+ struct publication *p, *tmpb;
+
+ spin_lock_bh(&sc->lock);
+ rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
+ list_for_each_entry_safe(p, tmpb,
+ &sr->all_publ, all_publ) {
+ tipc_service_remove_publ(net, sc, p->lower, p->node,
+ p->port, p->key);
+ kfree_rcu(p, rcu);
+ }
}
- hlist_del_init_rcu(&seq->ns_list);
- kfree(seq->sseqs);
- spin_unlock_bh(&seq->lock);
-
- kfree_rcu(seq, rcu);
+ hlist_del_init_rcu(&sc->service_list);
+ spin_unlock_bh(&sc->lock);
+ kfree_rcu(sc, rcu);
}
void tipc_nametbl_stop(struct net *net)
{
+ struct name_table *nt = tipc_name_table(net);
+ struct tipc_net *tn = tipc_net(net);
+ struct hlist_head *service_head;
+ struct tipc_service *service;
u32 i;
- struct name_seq *seq;
- struct hlist_head *seq_head;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct name_table *tipc_nametbl = tn->nametbl;
/* Verify name table is empty and purge any lingering
* publications, then release the name table
*/
spin_lock_bh(&tn->nametbl_lock);
for (i = 0; i < TIPC_NAMETBL_SIZE; i++) {
- if (hlist_empty(&tipc_nametbl->seq_hlist[i]))
+ if (hlist_empty(&nt->services[i]))
continue;
- seq_head = &tipc_nametbl->seq_hlist[i];
- hlist_for_each_entry_rcu(seq, seq_head, ns_list) {
- tipc_purge_publications(net, seq);
+ service_head = &nt->services[i];
+ hlist_for_each_entry_rcu(service, service_head, service_list) {
+ tipc_service_delete(net, service);
}
}
spin_unlock_bh(&tn->nametbl_lock);
synchronize_net();
- kfree(tipc_nametbl);
-
+ kfree(nt);
}
static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
- struct name_seq *seq,
- struct sub_seq *sseq, u32 *last_publ)
+ struct tipc_service *service,
+ struct service_range *sr,
+ u32 *last_key)
{
- void *hdr;
- struct nlattr *attrs;
- struct nlattr *publ;
struct publication *p;
+ struct nlattr *attrs;
+ struct nlattr *b;
+ void *hdr;
- if (*last_publ) {
- list_for_each_entry(p, &sseq->info->all_publ, all_publ)
- if (p->key == *last_publ)
+ if (*last_key) {
+ list_for_each_entry(p, &sr->all_publ, all_publ)
+ if (p->key == *last_key)
break;
- if (p->key != *last_publ)
+ if (p->key != *last_key)
return -EPIPE;
} else {
- p = list_first_entry(&sseq->info->all_publ, struct publication,
+ p = list_first_entry(&sr->all_publ,
+ struct publication,
all_publ);
}
- list_for_each_entry_from(p, &sseq->info->all_publ, all_publ) {
- *last_publ = p->key;
+ list_for_each_entry_from(p, &sr->all_publ, all_publ) {
+ *last_key = p->key;
hdr = genlmsg_put(msg->skb, msg->portid, msg->seq,
&tipc_genl_family, NLM_F_MULTI,
@@ -925,15 +836,15 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
if (!attrs)
goto msg_full;
- publ = nla_nest_start(msg->skb, TIPC_NLA_NAME_TABLE_PUBL);
- if (!publ)
+ b = nla_nest_start(msg->skb, TIPC_NLA_NAME_TABLE_PUBL);
+ if (!b)
goto attr_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_TYPE, seq->type))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_TYPE, service->type))
goto publ_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_LOWER, sseq->lower))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_LOWER, sr->lower))
goto publ_msg_full;
- if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_UPPER, sseq->upper))
+ if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_UPPER, sr->upper))
goto publ_msg_full;
if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_SCOPE, p->scope))
goto publ_msg_full;
@@ -944,16 +855,16 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
if (nla_put_u32(msg->skb, TIPC_NLA_PUBL_KEY, p->key))
goto publ_msg_full;
- nla_nest_end(msg->skb, publ);
+ nla_nest_end(msg->skb, b);
nla_nest_end(msg->skb, attrs);
genlmsg_end(msg->skb, hdr);
}
- *last_publ = 0;
+ *last_key = 0;
return 0;
publ_msg_full:
- nla_nest_cancel(msg->skb, publ);
+ nla_nest_cancel(msg->skb, b);
attr_msg_full:
nla_nest_cancel(msg->skb, attrs);
msg_full:
@@ -962,39 +873,34 @@ static int __tipc_nl_add_nametable_publ(struct tipc_nl_msg *msg,
return -EMSGSIZE;
}
-static int __tipc_nl_subseq_list(struct tipc_nl_msg *msg, struct name_seq *seq,
- u32 *last_lower, u32 *last_publ)
+static int __tipc_nl_service_range_list(struct tipc_nl_msg *msg,
+ struct tipc_service *sc,
+ u32 *last_lower, u32 *last_key)
{
- struct sub_seq *sseq;
- struct sub_seq *sseq_start;
+ struct service_range *sr;
+ struct rb_node *n;
int err;
- if (*last_lower) {
- sseq_start = nameseq_find_subseq(seq, *last_lower);
- if (!sseq_start)
- return -EPIPE;
- } else {
- sseq_start = seq->sseqs;
- }
-
- for (sseq = sseq_start; sseq != &seq->sseqs[seq->first_free]; sseq++) {
- err = __tipc_nl_add_nametable_publ(msg, seq, sseq, last_publ);
+ for (n = rb_first(&sc->ranges); n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->lower < *last_lower)
+ continue;
+ err = __tipc_nl_add_nametable_publ(msg, sc, sr, last_key);
if (err) {
- *last_lower = sseq->lower;
+ *last_lower = sr->lower;
return err;
}
}
*last_lower = 0;
-
return 0;
}
-static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
- u32 *last_type, u32 *last_lower, u32 *last_publ)
+static int tipc_nl_service_list(struct net *net, struct tipc_nl_msg *msg,
+ u32 *last_type, u32 *last_lower, u32 *last_key)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- struct hlist_head *seq_head;
- struct name_seq *seq = NULL;
+ struct tipc_net *tn = tipc_net(net);
+ struct tipc_service *service = NULL;
+ struct hlist_head *head;
int err;
int i;
@@ -1004,30 +910,31 @@ static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
i = 0;
for (; i < TIPC_NAMETBL_SIZE; i++) {
- seq_head = &tn->nametbl->seq_hlist[i];
+ head = &tn->nametbl->services[i];
if (*last_type) {
- seq = nametbl_find_seq(net, *last_type);
- if (!seq)
+ service = tipc_service_find(net, *last_type);
+ if (!service)
return -EPIPE;
} else {
- hlist_for_each_entry_rcu(seq, seq_head, ns_list)
+ hlist_for_each_entry_rcu(service, head, service_list)
break;
- if (!seq)
+ if (!service)
continue;
}
- hlist_for_each_entry_from_rcu(seq, ns_list) {
- spin_lock_bh(&seq->lock);
- err = __tipc_nl_subseq_list(msg, seq, last_lower,
- last_publ);
+ hlist_for_each_entry_from_rcu(service, service_list) {
+ spin_lock_bh(&service->lock);
+ err = __tipc_nl_service_range_list(msg, service,
+ last_lower,
+ last_key);
if (err) {
- *last_type = seq->type;
- spin_unlock_bh(&seq->lock);
+ *last_type = service->type;
+ spin_unlock_bh(&service->lock);
return err;
}
- spin_unlock_bh(&seq->lock);
+ spin_unlock_bh(&service->lock);
}
*last_type = 0;
}
@@ -1036,13 +943,13 @@ static int tipc_nl_seq_list(struct net *net, struct tipc_nl_msg *msg,
int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
{
- int err;
- int done = cb->args[3];
+ struct net *net = sock_net(skb->sk);
u32 last_type = cb->args[0];
u32 last_lower = cb->args[1];
- u32 last_publ = cb->args[2];
- struct net *net = sock_net(skb->sk);
+ u32 last_key = cb->args[2];
+ int done = cb->args[3];
struct tipc_nl_msg msg;
+ int err;
if (done)
return 0;
@@ -1052,7 +959,8 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
msg.seq = cb->nlh->nlmsg_seq;
rcu_read_lock();
- err = tipc_nl_seq_list(net, &msg, &last_type, &last_lower, &last_publ);
+ err = tipc_nl_service_list(net, &msg, &last_type,
+ &last_lower, &last_key);
if (!err) {
done = 1;
} else if (err != -EMSGSIZE) {
@@ -1068,7 +976,7 @@ int tipc_nl_name_table_dump(struct sk_buff *skb, struct netlink_callback *cb)
cb->args[0] = last_type;
cb->args[1] = last_lower;
- cb->args[2] = last_publ;
+ cb->args[2] = last_key;
cb->args[3] = done;
return skb->len;
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 34a4ccb..1b03b87 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -97,7 +97,7 @@ struct publication {
* @local_publ_count: number of publications issued by this node
*/
struct name_table {
- struct hlist_head seq_hlist[TIPC_NAMETBL_SIZE];
+ struct hlist_head services[TIPC_NAMETBL_SIZE];
struct list_head node_scope;
struct list_head cluster_scope;
u32 local_publ_count;
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 4fb4327..85e777e 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -324,12 +324,12 @@ static void tipc_node_write_unlock(struct tipc_node *n)
if (flags & TIPC_NOTIFY_LINK_UP) {
tipc_mon_peer_up(net, addr, bearer_id);
tipc_nametbl_publish(net, TIPC_LINK_STATE, addr, addr,
- TIPC_NODE_SCOPE, link_id, addr);
+ TIPC_NODE_SCOPE, link_id, link_id);
}
if (flags & TIPC_NOTIFY_LINK_DOWN) {
tipc_mon_peer_down(net, addr, bearer_id);
tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
- link_id, addr);
+ link_id, link_id);
}
}
diff --git a/net/tipc/subscr.h b/net/tipc/subscr.h
index 8b2d22b..d793b43 100644
--- a/net/tipc/subscr.h
+++ b/net/tipc/subscr.h
@@ -40,7 +40,7 @@
#include "topsrv.h"
#define TIPC_MAX_SUBSCR 65535
-#define TIPC_MAX_PUBLICATIONS 65535
+#define TIPC_MAX_PUBL 65535
struct tipc_subscription;
struct tipc_conn;
@@ -58,7 +58,7 @@ struct tipc_subscription {
struct kref kref;
struct net *net;
struct timer_list timer;
- struct list_head nameseq_list;
+ struct list_head service_list;
struct list_head sub_list;
struct tipc_event evt;
int conid;
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [net-next 4/4] tipc: tipc: rename address types in user api
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev; +Cc: tipc-discussion, mohan.krishna.ghanta.krishnamurthy
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The three address type structs in the user API have names that in
reality reflect the specific, non-Linux environment where they were
originally created.
We now give them more intuitive names, in accordance with how TIPC is
described in the current documentation.
struct tipc_portid -> struct tipc_socket_addr
struct tipc_name -> struct tipc_service_addr
struct tipc_name_seq -> struct tipc_service_range
For compatibility, we add macros that map the old names to the new ones.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
include/uapi/linux/tipc.h | 32 ++++++++++++++++++--------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/include/uapi/linux/tipc.h b/include/uapi/linux/tipc.h
index 4ac9f1f..254f6e3 100644
--- a/include/uapi/linux/tipc.h
+++ b/include/uapi/linux/tipc.h
@@ -45,17 +45,17 @@
* TIPC addressing primitives
*/
-struct tipc_portid {
+struct tipc_socket_addr {
__u32 ref;
__u32 node;
};
-struct tipc_name {
+struct tipc_service_addr {
__u32 type;
__u32 instance;
};
-struct tipc_name_seq {
+struct tipc_service_range {
__u32 type;
__u32 lower;
__u32 upper;
@@ -108,28 +108,28 @@ enum tipc_scope {
* TIPC topology subscription service definitions
*/
-#define TIPC_SUB_PORTS 0x01 /* filter for port availability */
-#define TIPC_SUB_SERVICE 0x02 /* filter for service availability */
-#define TIPC_SUB_CANCEL 0x04 /* cancel a subscription */
+#define TIPC_SUB_PORTS 0x01 /* filter: evt at each match */
+#define TIPC_SUB_SERVICE 0x02 /* filter: evt at first up/last down */
+#define TIPC_SUB_CANCEL 0x04 /* filter: cancel a subscription */
#define TIPC_WAIT_FOREVER (~0) /* timeout for permanent subscription */
struct tipc_subscr {
- struct tipc_name_seq seq; /* name sequence of interest */
+ struct tipc_service_range seq; /* range of interest */
__u32 timeout; /* subscription duration (in ms) */
__u32 filter; /* bitmask of filter options */
char usr_handle[8]; /* available for subscriber use */
};
#define TIPC_PUBLISHED 1 /* publication event */
-#define TIPC_WITHDRAWN 2 /* withdraw event */
+#define TIPC_WITHDRAWN 2 /* withdrawal event */
#define TIPC_SUBSCR_TIMEOUT 3 /* subscription timeout event */
struct tipc_event {
__u32 event; /* event type */
- __u32 found_lower; /* matching name seq instances */
- __u32 found_upper; /* " " " " */
- struct tipc_portid port; /* associated port */
+ __u32 found_lower; /* matching range */
+ __u32 found_upper; /* " " */
+ struct tipc_socket_addr port; /* associated socket */
struct tipc_subscr s; /* associated subscription */
};
@@ -159,10 +159,10 @@ struct sockaddr_tipc {
unsigned char addrtype;
signed char scope;
union {
- struct tipc_portid id;
- struct tipc_name_seq nameseq;
+ struct tipc_socket_addr id;
+ struct tipc_service_range nameseq;
struct {
- struct tipc_name name;
+ struct tipc_service_addr name;
__u32 domain;
} name;
} addr;
@@ -250,6 +250,10 @@ struct tipc_sioc_ln_req {
#define TIPC_ZONE_CLUSTER_MASK (TIPC_ZONE_MASK | TIPC_CLUSTER_MASK)
+#define tipc_portid tipc_socket_addr
+#define tipc_name tipc_service_addr
+#define tipc_name_seq tipc_service_range
+
static inline __u32 tipc_addr(unsigned int zone,
unsigned int cluster,
unsigned int node)
--
2.1.4
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply related
* [net-next 2/4] tipc: refactor name table translate function
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
jon.maloy, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
The function tipc_nametbl_translate() function is ugly and hard to
follow. This can be improved somewhat by introducing a stack variable
for holding the publication list to be used and re-ordering the if-
clauses for selection of algorithm.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_table.c | 61 +++++++++++++++++++++------------------------------
1 file changed, 25 insertions(+), 36 deletions(-)
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index c309402..9915be0 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -399,29 +399,32 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
/**
* tipc_nametbl_translate - perform service instance to socket translation
*
- * On entry, 'destnode' is the search domain used during translation.
+ * On entry, 'dnode' is the search domain used during translation.
*
* On exit:
- * - if name translation is deferred to another node/cluster/zone,
- * leaves 'destnode' unchanged (will be non-zero) and returns 0
- * - if name translation is attempted and succeeds, sets 'destnode'
- * to publication node and returns port reference (will be non-zero)
- * - if name translation is attempted and fails, sets 'destnode' to 0
- * and returns 0
+ * - if translation is deferred to another node, leave 'dnode' unchanged and
+ * return 0
+ * - if translation is attempted and succeeds, set 'dnode' to the publishing
+ * node and return the published (non-zero) port number
+ * - if translation is attempted and fails, set 'dnode' to 0 and return 0
+ *
+ * Note that for legacy users (node configured with Z.C.N address format) the
+ * 'closest-first' lookup algorithm must be maintained, i.e., if dnode is 0
+ * we must look in the local binding list first
*/
-u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
- u32 *destnode)
+u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance, u32 *dnode)
{
struct tipc_net *tn = tipc_net(net);
bool legacy = tn->legacy_addr_format;
u32 self = tipc_own_addr(net);
struct service_range *sr;
struct tipc_service *sc;
+ struct list_head *list;
struct publication *p;
u32 port = 0;
u32 node = 0;
- if (!tipc_in_scope(legacy, *destnode, self))
+ if (!tipc_in_scope(legacy, *dnode, self))
return 0;
rcu_read_lock();
@@ -434,43 +437,29 @@ u32 tipc_nametbl_translate(struct net *net, u32 type, u32 instance,
if (unlikely(!sr))
goto no_match;
- /* Closest-First Algorithm */
- if (legacy && !*destnode) {
- if (!list_empty(&sr->local_publ)) {
- p = list_first_entry(&sr->local_publ,
- struct publication,
- local_publ);
- list_move_tail(&p->local_publ,
- &sr->local_publ);
- } else {
- p = list_first_entry(&sr->all_publ,
- struct publication,
- all_publ);
- list_move_tail(&p->all_publ,
- &sr->all_publ);
- }
- }
-
- /* Round-Robin Algorithm */
- else if (*destnode == self) {
- if (list_empty(&sr->local_publ))
+ /* Select lookup algorithm: local, closest-first or round-robin */
+ if (*dnode == self) {
+ list = &sr->local_publ;
+ if (list_empty(list))
goto no_match;
- p = list_first_entry(&sr->local_publ, struct publication,
- local_publ);
+ p = list_first_entry(list, struct publication, local_publ);
+ list_move_tail(&p->local_publ, &sr->local_publ);
+ } else if (legacy && !*dnode && !list_empty(&sr->local_publ)) {
+ list = &sr->local_publ;
+ p = list_first_entry(list, struct publication, local_publ);
list_move_tail(&p->local_publ, &sr->local_publ);
} else {
- p = list_first_entry(&sr->all_publ, struct publication,
- all_publ);
+ list = &sr->all_publ;
+ p = list_first_entry(list, struct publication, all_publ);
list_move_tail(&p->all_publ, &sr->all_publ);
}
-
port = p->port;
node = p->node;
no_match:
spin_unlock_bh(&sc->lock);
not_found:
rcu_read_unlock();
- *destnode = node;
+ *dnode = node;
return port;
}
--
2.1.4
^ permalink raw reply related
* [net-next 3/4] tipc: permit overlapping service ranges in name table
From: Jon Maloy @ 2018-03-28 14:58 UTC (permalink / raw)
To: davem, netdev
Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
jon.maloy, canh.d.luu, ying.xue, tipc-discussion
In-Reply-To: <1522249103-32714-1-git-send-email-jon.maloy@ericsson.com>
With the new RB tree structure for service ranges it becomes possible to
solve an old problem; - we can now allow overlapping service ranges in
the table.
When inserting a new service range to the tree, we use 'lower' as primary
key, and when necessary 'upper' as secondary key.
Since there may now be multiple service ranges matching an indicated
'lower' value, we must also add the 'upper' value to the functions
used for removing publications, so that the correct, corresponding
range item can be found.
These changes guarantee that a well-formed publication/withdrawal item
from a peer node never will be rejected, and make it possible to
eliminate the problematic backlog functionality we currently have for
handling such cases.
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
net/tipc/name_distr.c | 90 +++++++++++++--------------------------------------
net/tipc/name_distr.h | 1 -
net/tipc/name_table.c | 64 +++++++++++++++++-------------------
net/tipc/name_table.h | 8 ++---
net/tipc/net.c | 2 +-
net/tipc/node.c | 2 +-
net/tipc/socket.c | 4 +--
7 files changed, 60 insertions(+), 111 deletions(-)
diff --git a/net/tipc/name_distr.c b/net/tipc/name_distr.c
index 8240a85..51b4b96 100644
--- a/net/tipc/name_distr.c
+++ b/net/tipc/name_distr.c
@@ -204,12 +204,12 @@ void tipc_named_node_up(struct net *net, u32 dnode)
*/
static void tipc_publ_purge(struct net *net, struct publication *publ, u32 addr)
{
- struct tipc_net *tn = net_generic(net, tipc_net_id);
+ struct tipc_net *tn = tipc_net(net);
struct publication *p;
spin_lock_bh(&tn->nametbl_lock);
- p = tipc_nametbl_remove_publ(net, publ->type, publ->lower,
- publ->node, publ->port, publ->key);
+ p = tipc_nametbl_remove_publ(net, publ->type, publ->lower, publ->upper,
+ publ->node, publ->key);
if (p)
tipc_node_unsubscribe(net, &p->binding_node, addr);
spin_unlock_bh(&tn->nametbl_lock);
@@ -261,28 +261,31 @@ void tipc_publ_notify(struct net *net, struct list_head *nsub_list, u32 addr)
static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
u32 node, u32 dtype)
{
- struct publication *publ = NULL;
+ struct publication *p = NULL;
+ u32 lower = ntohl(i->lower);
+ u32 upper = ntohl(i->upper);
+ u32 type = ntohl(i->type);
+ u32 port = ntohl(i->port);
+ u32 key = ntohl(i->key);
if (dtype == PUBLICATION) {
- publ = tipc_nametbl_insert_publ(net, ntohl(i->type),
- ntohl(i->lower),
- ntohl(i->upper),
- TIPC_CLUSTER_SCOPE, node,
- ntohl(i->port), ntohl(i->key));
- if (publ) {
- tipc_node_subscribe(net, &publ->binding_node, node);
+ p = tipc_nametbl_insert_publ(net, type, lower, upper,
+ TIPC_CLUSTER_SCOPE, node,
+ port, key);
+ if (p) {
+ tipc_node_subscribe(net, &p->binding_node, node);
return true;
}
} else if (dtype == WITHDRAWAL) {
- publ = tipc_nametbl_remove_publ(net, ntohl(i->type),
- ntohl(i->lower),
- node, ntohl(i->port),
- ntohl(i->key));
- if (publ) {
- tipc_node_unsubscribe(net, &publ->binding_node, node);
- kfree_rcu(publ, rcu);
+ p = tipc_nametbl_remove_publ(net, type, lower,
+ upper, node, key);
+ if (p) {
+ tipc_node_unsubscribe(net, &p->binding_node, node);
+ kfree_rcu(p, rcu);
return true;
}
+ pr_warn_ratelimited("Failed to remove binding %u,%u from %x\n",
+ type, lower, node);
} else {
pr_warn("Unrecognized name table message received\n");
}
@@ -290,53 +293,6 @@ static bool tipc_update_nametbl(struct net *net, struct distr_item *i,
}
/**
- * tipc_named_add_backlog - add a failed name table update to the backlog
- *
- */
-static void tipc_named_add_backlog(struct net *net, struct distr_item *i,
- u32 type, u32 node)
-{
- struct distr_queue_item *e;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- unsigned long now = get_jiffies_64();
-
- e = kzalloc(sizeof(*e), GFP_ATOMIC);
- if (!e)
- return;
- e->dtype = type;
- e->node = node;
- e->expires = now + msecs_to_jiffies(sysctl_tipc_named_timeout);
- memcpy(e, i, sizeof(*i));
- list_add_tail(&e->next, &tn->dist_queue);
-}
-
-/**
- * tipc_named_process_backlog - try to process any pending name table updates
- * from the network.
- */
-void tipc_named_process_backlog(struct net *net)
-{
- struct distr_queue_item *e, *tmp;
- struct tipc_net *tn = net_generic(net, tipc_net_id);
- unsigned long now = get_jiffies_64();
-
- list_for_each_entry_safe(e, tmp, &tn->dist_queue, next) {
- if (time_after(e->expires, now)) {
- if (!tipc_update_nametbl(net, &e->i, e->node, e->dtype))
- continue;
- } else {
- pr_warn_ratelimited("Dropping name table update (%d) of {%u, %u, %u} from %x key=%u\n",
- e->dtype, ntohl(e->i.type),
- ntohl(e->i.lower),
- ntohl(e->i.upper),
- e->node, ntohl(e->i.key));
- }
- list_del(&e->next);
- kfree(e);
- }
-}
-
-/**
* tipc_named_rcv - process name table update messages sent by another node
*/
void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
@@ -358,12 +314,10 @@ void tipc_named_rcv(struct net *net, struct sk_buff_head *inputq)
count = msg_data_sz(msg) / ITEM_SIZE;
node = msg_orignode(msg);
while (count--) {
- if (!tipc_update_nametbl(net, item, node, mtype))
- tipc_named_add_backlog(net, item, mtype, node);
+ tipc_update_nametbl(net, item, node, mtype);
item++;
}
kfree_skb(skb);
- tipc_named_process_backlog(net);
}
spin_unlock_bh(&tn->nametbl_lock);
}
diff --git a/net/tipc/name_distr.h b/net/tipc/name_distr.h
index 4753e62..63fc73e 100644
--- a/net/tipc/name_distr.h
+++ b/net/tipc/name_distr.h
@@ -72,7 +72,6 @@ struct sk_buff *tipc_named_withdraw(struct net *net, struct publication *publ);
void tipc_named_node_up(struct net *net, u32 dnode);
void tipc_named_rcv(struct net *net, struct sk_buff_head *msg_queue);
void tipc_named_reinit(struct net *net);
-void tipc_named_process_backlog(struct net *net);
void tipc_publ_notify(struct net *net, struct list_head *nsub_list, u32 addr);
#endif
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index 9915be0..bba3e41 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -171,10 +171,14 @@ static struct service_range *tipc_service_create_range(struct tipc_service *sc,
tmp = container_of(parent, struct service_range, tree_node);
if (lower < tmp->lower)
n = &(*n)->rb_left;
+ else if (lower > tmp->lower)
+ n = &(*n)->rb_right;
+ else if (upper < tmp->upper)
+ n = &(*n)->rb_left;
else if (upper > tmp->upper)
n = &(*n)->rb_right;
else
- return NULL;
+ return tmp;
}
sr = kzalloc(sizeof(*sr), GFP_ATOMIC);
if (!sr)
@@ -200,17 +204,11 @@ static struct publication *tipc_service_insert_publ(struct net *net,
struct publication *p;
bool first = false;
- sr = tipc_service_find_range(sc, lower);
- if (!sr) {
- sr = tipc_service_create_range(sc, lower, upper);
- if (!sr)
- goto err;
- first = true;
- }
+ sr = tipc_service_create_range(sc, lower, upper);
+ if (!sr)
+ goto err;
- /* Lower end overlaps existing entry, but we need an exact match */
- if (sr->lower != lower || sr->upper != upper)
- return NULL;
+ first = list_empty(&sr->all_publ);
/* Return if the publication already exists */
list_for_each_entry(p, &sr->all_publ, all_publ) {
@@ -239,30 +237,32 @@ static struct publication *tipc_service_insert_publ(struct net *net,
/**
* tipc_service_remove_publ - remove a publication from a service
- *
- * NOTE: There may be cases where TIPC is asked to remove a publication
- * that is not in the name table. For example, if another node issues a
- * publication for a name range that overlaps an existing name range
- * the publication will not be recorded, which means the publication won't
- * be found when the name range is later withdrawn by that node.
- * A failed withdraw request simply returns a failure indication and lets the
- * caller issue any error or warning messages associated with such a problem.
*/
static struct publication *tipc_service_remove_publ(struct net *net,
struct tipc_service *sc,
- u32 inst, u32 node,
- u32 port, u32 key)
+ u32 lower, u32 upper,
+ u32 node, u32 key)
{
struct tipc_subscription *sub, *tmp;
struct service_range *sr;
struct publication *p;
bool found = false;
bool last = false;
+ struct rb_node *n;
- sr = tipc_service_find_range(sc, inst);
+ sr = tipc_service_find_range(sc, lower);
if (!sr)
return NULL;
+ /* Find exact matching service range */
+ for (n = &sr->tree_node; n; n = rb_next(n)) {
+ sr = container_of(n, struct service_range, tree_node);
+ if (sr->upper == upper)
+ break;
+ }
+ if (!n || sr->lower != lower || sr->upper != upper)
+ return NULL;
+
/* Find publication, if it exists */
list_for_each_entry(p, &sr->all_publ, all_publ) {
if (p->key != key || (node && node != p->node))
@@ -375,8 +375,8 @@ struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
}
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 port,
- u32 key)
+ u32 lower, u32 upper,
+ u32 node, u32 key)
{
struct tipc_service *sc = tipc_service_find(net, type);
struct publication *p = NULL;
@@ -385,7 +385,7 @@ struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
return NULL;
spin_lock_bh(&sc->lock);
- p = tipc_service_remove_publ(net, sc, lower, node, port, key);
+ p = tipc_service_remove_publ(net, sc, lower, upper, node, key);
/* Delete service item if this no more publications and subscriptions */
if (RB_EMPTY_ROOT(&sc->ranges) && list_empty(&sc->subscriptions)) {
@@ -620,8 +620,6 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
if (p) {
nt->local_publ_count++;
skb = tipc_named_publish(net, p);
- /* Any pending external events? */
- tipc_named_process_backlog(net);
}
exit:
spin_unlock_bh(&tn->nametbl_lock);
@@ -635,7 +633,7 @@ struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
* tipc_nametbl_withdraw - withdraw a service binding
*/
int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
- u32 port, u32 key)
+ u32 upper, u32 key)
{
struct name_table *nt = tipc_name_table(net);
struct tipc_net *tn = tipc_net(net);
@@ -645,17 +643,15 @@ int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower,
spin_lock_bh(&tn->nametbl_lock);
- p = tipc_nametbl_remove_publ(net, type, lower, self, port, key);
+ p = tipc_nametbl_remove_publ(net, type, lower, upper, self, key);
if (p) {
nt->local_publ_count--;
skb = tipc_named_withdraw(net, p);
- /* Any pending external events? */
- tipc_named_process_backlog(net);
list_del_init(&p->binding_sock);
kfree_rcu(p, rcu);
} else {
pr_err("Failed to remove local publication {%u,%u,%u}/%u\n",
- type, lower, port, key);
+ type, lower, upper, key);
}
spin_unlock_bh(&tn->nametbl_lock);
@@ -754,8 +750,8 @@ static void tipc_service_delete(struct net *net, struct tipc_service *sc)
rbtree_postorder_for_each_entry_safe(sr, tmpr, &sc->ranges, tree_node) {
list_for_each_entry_safe(p, tmpb,
&sr->all_publ, all_publ) {
- tipc_service_remove_publ(net, sc, p->lower, p->node,
- p->port, p->key);
+ tipc_service_remove_publ(net, sc, p->lower, p->upper,
+ p->node, p->key);
kfree_rcu(p, rcu);
}
}
diff --git a/net/tipc/name_table.h b/net/tipc/name_table.h
index 1b03b87..4b14fc2 100644
--- a/net/tipc/name_table.h
+++ b/net/tipc/name_table.h
@@ -116,16 +116,16 @@ bool tipc_nametbl_lookup(struct net *net, u32 type, u32 instance, u32 domain,
struct list_head *dsts, int *dstcnt, u32 exclude,
bool all);
struct publication *tipc_nametbl_publish(struct net *net, u32 type, u32 lower,
- u32 upper, u32 scope, u32 port_ref,
+ u32 upper, u32 scope, u32 port,
u32 key);
-int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 ref,
+int tipc_nametbl_withdraw(struct net *net, u32 type, u32 lower, u32 upper,
u32 key);
struct publication *tipc_nametbl_insert_publ(struct net *net, u32 type,
u32 lower, u32 upper, u32 scope,
u32 node, u32 ref, u32 key);
struct publication *tipc_nametbl_remove_publ(struct net *net, u32 type,
- u32 lower, u32 node, u32 ref,
- u32 key);
+ u32 lower, u32 upper,
+ u32 node, u32 key);
void tipc_nametbl_subscribe(struct tipc_subscription *s);
void tipc_nametbl_unsubscribe(struct tipc_subscription *s);
int tipc_nametbl_init(struct net *net);
diff --git a/net/tipc/net.c b/net/tipc/net.c
index 29538dc..856f9e9 100644
--- a/net/tipc/net.c
+++ b/net/tipc/net.c
@@ -136,7 +136,7 @@ void tipc_net_stop(struct net *net)
if (!self)
return;
- tipc_nametbl_withdraw(net, TIPC_CFG_SRV, self, 0, self);
+ tipc_nametbl_withdraw(net, TIPC_CFG_SRV, self, self, self);
rtnl_lock();
tipc_bearer_stop(net);
tipc_node_stop(net);
diff --git a/net/tipc/node.c b/net/tipc/node.c
index 85e777e..c77dd2f 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -329,7 +329,7 @@ static void tipc_node_write_unlock(struct tipc_node *n)
if (flags & TIPC_NOTIFY_LINK_DOWN) {
tipc_mon_peer_down(net, addr, bearer_id);
tipc_nametbl_withdraw(net, TIPC_LINK_STATE, addr,
- link_id, link_id);
+ addr, link_id);
}
}
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 275b666..3e5eba3 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -2634,12 +2634,12 @@ static int tipc_sk_withdraw(struct tipc_sock *tsk, uint scope,
if (publ->upper != seq->upper)
break;
tipc_nametbl_withdraw(net, publ->type, publ->lower,
- publ->port, publ->key);
+ publ->upper, publ->key);
rc = 0;
break;
}
tipc_nametbl_withdraw(net, publ->type, publ->lower,
- publ->port, publ->key);
+ publ->upper, publ->key);
rc = 0;
}
if (list_empty(&tsk->publications))
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-28 15:04 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <CAK6E8=f77S5VjSofC6v8EY2U+YQ0incyv7beKA-aP2p9c7ebUg@mail.gmail.com>
On Wed, Mar 28, 2018 at 7:14 AM, Yuchung Cheng <ycheng@google.com> wrote:
>
> On Wed, Mar 28, 2018 at 5:45 AM, Ilpo Järvinen
> <ilpo.jarvinen@helsinki.fi> wrote:
> > On Tue, 27 Mar 2018, Yuchung Cheng wrote:
> >
> >> On Tue, Mar 27, 2018 at 7:23 AM, Ilpo Järvinen
> >> <ilpo.jarvinen@helsinki.fi> wrote:
> >> > On Mon, 26 Mar 2018, Yuchung Cheng wrote:
> >> >
> >> >> On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
> >> >> <ilpo.jarvinen@helsinki.fi> wrote:
> >> >> >
> >> >> > A miscalculation for the number of acknowledged packets occurs during
> >> >> > RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> >> >> > any non-retransmitted skbs. The reason is that pkts_acked value
> >> >> > calculated in tcp_clean_rtx_queue is not correct for slow start after
> >> >> > RTO as it may include segments that were not lost and therefore did
> >> >> > not need retransmissions in the slow start following the RTO. Then
> >> >> > tcp_slow_start will add the excess into cwnd bloating it and
> >> >> > triggering a burst.
> >> >> >
> >> >> > Instead, we want to pass only the number of retransmitted segments
> >> >> > that were covered by the cumulative ACK (and potentially newly sent
> >> >> > data segments too if the cumulative ACK covers that far).
> >> >> >
> >> >> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> >> >> > ---
> >> >>
> >> >> My understanding is there are two problems
> >> >>
> >> >> 1) your fix: the reordering logic in tcp-remove_reno_sacks requires
> >> >> precise cumulatively acked count, not newly acked count?
> >> >
> >> > While I'm not entirely sure if you intented to say that my fix is broken
> >> > or not, I thought this very difference alot while making the fix and I
> >> > believe that this fix is needed because of the discontinuity at RTO
> >> > (sacked_out is cleared as we set L-bits + lost_out). This is an artifact
> >> > in the imitation of sacked_out for non-SACK but at RTO we can't keep that
> >> > in sync because we set L-bits (and have no S-bits to guide us). Thus, we
> >> > cannot anymore "use" those skbs with only L-bit for the reno_sacks logic.
> >> >
> >> > In tcp_remove_reno_sacks acked - sacked_out is being used to calculate
> >> > tp->delivered, using plain cumulative acked causes congestion control
> >> > breakage later as call to tcp_cong_control will directly use the
> >> > difference in tp->delivered.
> >> >
> >> > This boils down the exact definition of tp->delivered (the one given in
> >> > the header is not detailed enough). I guess you might have better idea
> >> > what it exactly is since one of you has added it? There are subtle things
> >> > in the defination that can make it entirely unsuitable for cc decisions.
> >> > Should those segments that we (possibly) already counted into
> >> > tp->delivered during (potentially preceeding) CA_Recovery be added to it
> >> > for _second time_ or not? This fix avoids such double counting (the
> >> Where is the double counting, assuming normal DUPACK behavior?
> >>
> >> In the non-sack case:
> >>
> >> 1. upon receiving a DUPACK, we assume one packet has been delivered by
> >> incrementing tp->delivered in tcp_add_reno_sack()
> >
> > 1b. RTO here. We clear tp->sacked_out at RTO (i.e., the discontinuity
> > I've tried to point out quite many times already)...
> >
> >> 2. upon receiving a partial ACK or an ACK that acks recovery point
> >> (high_seq), tp->delivered is incremented by (cumulatively acked -
> >> #dupacks) in tcp_remove_reno_sacks()
> >
> > ...and this won't happen correctly anymore after RTO (since non-SACK
> > won't keep #dupacks due to the discontinuity). Thus we end up adding
> > cumulatively acked - 0 to tp->delivered on those ACKs.
> >
> >> therefore tp->delivered is tracking the # of packets delivered
> >> (sacked, acked, DUPACK'd) with the most information it could have
> >> inferred.
> >
> > Since you didn't answer any of my questions about tp->delivered directly,
> > let me rephrase them to this example (non-SACK, of course):
> >
> > 4 segments outstanding. RTO recovery underway (lost_out=4, sacked_out=0).
> > Cwnd = 2 so the sender rexmits 2 out of 4. We get cumulative ACK for
> > three segments. How much should tp->delivered be incremented? 2 or 3?
> >
> > ...I think 2 is the right answer.
> >
> >> From congestion control's perspective, it cares about the delivery
> >> information (e.g. how much), not the sequences (what or how).
> >
> > I guess you must have missed my point. I'm talking about "how much"
> > whole the time. It's just about when can we account for it (and when not).
> >
> >> I am pointing out that
> >>
> >> 1) your fix may fix one corner CC packet accounting issue in the
> >> non-SACK and CA_Loss
> >> 2) but it does not fix the other (major) CC packet accounting issue in
> >> the SACK case
> >
> > You say major but it's major if and only if one is using one of the
> > affected cc modules which were not that many and IMHO not very mainstream
> > anyway (check my list from the previous email, not that I'd mind if they'd
> > get fixed too). The rest of the cc modules do not seem to use the incorrect
> > value even if some of them have the pkts_acked callback.
> >
> > Other CC packet accounting (regardless of SACK) is based on tp->delivered
> > and tp->delivered is NOT similarly miscounted with SACK because the logic
> > depend on !TCPCB_SACKED_ACKED (that fails only if we have very high ACK
> > loss).
> >
> >> 3) it breaks the dynamic dupthresh / reordering in the non-SACK case
> >> as tcp_check_reno_reordering requires strictly cum. ack.
> >
> > I think that the current non-SACK reordering detection logic is not that
> > sound after RTO (but I'm yet to prove this). ...There seems to be some
> > failure modes which is why I even thought of disabling the whole thing
> > for non-SACK RTO recoveries and as it now seems you do care more about
> > non-SACK than you initial claimed, I might even have motivation to fix
> > more those corners rather than the worst bugs only ;-). ...But I'll make
> > the tp->delivered fix only in this patch to avoid any change this part of
> > the code.
> >
> >> Therefore I prefer
> >> a) using tp->delivered to fix (1)(2)
> >> b) perhaps improving tp->delivered SACK emulation code in the non-SACK case
> >
> > Somehow I get an impression that you might assume/say here that
> [resending in plaintext]
> That's wrong impression. Perhaps it's worth re-iterating what I agree
> and disagree
>
> 1. [agree] there's accounting issue in non-SACK as you discovered
> which causes CC misbehavior
>
> 2. [major disagree] adjusting pkts_acked for ca_ops->pkts_acked in non-sack
> => that field is not used by common C.C. (you said so too)
>
> 3. [disagree] adjusting pkts_acked may not affect reordering
> accounting in non-sack
>
>
> For cubic or reno, the main source is the "acked_sacked" passed into
> tcp_cong_avoid(). that variable is derived from tp->delivered.
> Therefore we need to fix that to address the problem in (1)
>
> I have yet to read your code. Will read later today.
Your patch looks good. Some questions / comments:
1. Why only apply to CA_Loss and not also CA_Recovery? this may
mitigate GRO issue I noted in other thread.
2. minor style nit: can we adjust tp->delivered directly in
tcp_clean_rtx_queue(). I am personally against calling "non-sack" reno
(e.g. tcp_*reno*()) because its really confusing w/ Reno congestion
control when SACK is used. One day I'd like to rename all these *reno*
to _nonsack_.
Thanks
>
> > tp->delivered is all correct for non-SACK? ...It isn't without a fix.
> > Therefore a) won't help any for non-SACK. Tp->delivered for non-SACK is
> > currently (mis!)calculated in tcp_remove_reno_sacks because the incorrect
> > pkts_acked is fed to it. ...Thus, b) is an intermediate step in the
> > miscalculation chain I'm fixing with this change. B) resolves also (1)
> > without additional changes to logic!
> >
> > I've included below the updated change that only fixes tp->delivered
> > calculation (not tested besides compiling). ...But I think it's worse than
> > my previous version because tcp_remove_reno_sacks then assumes
> > non-sensical L|S skbs (but there seem to be other, worse limitations in
> > sacked_out imitation after RTO because we've all skbs marked with L-bit so
> > it's not that big deal for me as most of the that imitation code is no-op
> > anyway after RTO).
> >
> >
> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 9a1b3c1..e11748d 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -1868,7 +1868,6 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> >
> > if (acked > 0) {
> > /* One ACK acked hole. The rest eat duplicate ACKs. */
> > - tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> > if (acked - 1 >= tp->sacked_out)
> > tp->sacked_out = 0;
> > else
> > @@ -1878,6 +1877,12 @@ static void tcp_remove_reno_sacks(struct sock *sk, int acked)
> > tcp_verify_left_out(tp);
> > }
> >
> > +static void tcp_update_reno_delivered(struct tcp_sock *tp, int acked)
> > +{
> > + if (acked > 0)
> > + tp->delivered += max_t(int, acked - tp->sacked_out, 1);
> > +}
> > +
> > static inline void tcp_reset_reno_sack(struct tcp_sock *tp)
> > {
> > tp->sacked_out = 0;
> > @@ -3027,6 +3032,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > long seq_rtt_us = -1L;
> > long ca_rtt_us = -1L;
> > u32 pkts_acked = 0;
> > + u32 rexmit_acked = 0;
> > + u32 newdata_acked = 0;
> > u32 last_in_flight = 0;
> > bool rtt_update;
> > int flag = 0;
> > @@ -3056,8 +3063,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (unlikely(sacked & TCPCB_RETRANS)) {
> > - if (sacked & TCPCB_SACKED_RETRANS)
> > + if (sacked & TCPCB_SACKED_RETRANS) {
> > tp->retrans_out -= acked_pcount;
> > + rexmit_acked += acked_pcount;
> > + }
> > flag |= FLAG_RETRANS_DATA_ACKED;
> > } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> > last_ackt = skb->skb_mstamp;
> > @@ -3070,6 +3079,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > reord = start_seq;
> > if (!after(scb->end_seq, tp->high_seq))
> > flag |= FLAG_ORIG_SACK_ACKED;
> > + else
> > + newdata_acked += acked_pcount;
> > }
> >
> > if (sacked & TCPCB_SACKED_ACKED) {
> > @@ -3151,6 +3162,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> > }
> >
> > if (tcp_is_reno(tp)) {
> > + tcp_update_reno_delivered(tp, icsk->icsk_ca_state != TCP_CA_Loss ?
> > + pkts_acked :
> > + rexmit_acked + newdata_acked);
> > tcp_remove_reno_sacks(sk, pkts_acked);
> > } else {
> > int delta;
^ permalink raw reply
* Re: [RFC PATCH 00/24] Introducing AF_XDP support
From: William Tu @ 2018-03-28 15:05 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Björn Töpel, Karlsson, Magnus, Alexander Duyck,
Alexander Duyck, John Fastabend, Alexei Starovoitov,
Willem de Bruijn, Daniel Borkmann,
Linux Kernel Network Developers, Björn Töpel,
michael.lundkvist, Brandeburg, Jesse, Anjali Singhai Jain,
jeffrey.b.shaw, ferruh.yigit, qi.z.zhang, dendibakh
In-Reply-To: <20180328100136.6202448b@redhat.com>
Hi Jesper,
Thanks for the comments.
>> I assume this xdpsock code is small and should all fit into the icache.
>> However, doing another perf stat on xdpsock l2fwd shows
>>
>> 13,720,109,581 stalled-cycles-frontend # 60.01% frontend cycles
>> idle (23.82%)
>>
>> <not supported> stalled-cycles-backend
>> 7,994,837 branch-misses # 0.16% of all branches
>> (23.80%)
>> 996,874,424 bus-cycles # 99.679 M/sec (23.80%)
>> 18,942,220,445 ref-cycles # 1894.067 M/sec (28.56%)
>> 100,983,226 LLC-loads # 10.097 M/sec (23.80%)
>> 4,897,089 LLC-load-misses # 4.85% of all LL-cache hits (23.80%)
>> 66,659,889 LLC-stores # 6.665 M/sec (9.52%)
>> 8,373 LLC-store-misses # 0.837 K/sec (9.52%)
>> 158,178,410 LLC-prefetches # 15.817 M/sec (9.52%)
>> 3,011,180 LLC-prefetch-misses # 0.301 M/sec (9.52%)
>> 8,190,383,109 dTLB-loads # 818.971 M/sec (9.52%)
>> 20,432,204 dTLB-load-misses # 0.25% of all dTLB cache hits (9.52%)
>> 3,729,504,674 dTLB-stores # 372.920 M/sec (9.52%)
>> 992,231 dTLB-store-misses # 0.099 M/sec (9.52%)
>> <not supported> dTLB-prefetches
>> <not supported> dTLB-prefetch-misses
>> 11,619 iTLB-loads # 0.001 M/sec (9.52%)
>> 1,874,756 iTLB-load-misses # 16135.26% of all iTLB cache hits (14.28%)
>
> What was the sample period for this perf stat?
>
10 seconds.
root@ovs-smartnic:~/net-next/tools/perf# ./perf stat -C 6 sleep 10
>> I have super high iTLB-load-misses. This is probably the cause of high
>> frontend stalled.
>
> It looks very strange that your iTLB-loads are 11,619, while the
> iTLB-load-misses are much much higher 1,874,756.
>
Does it mean cpu try to load the code, then fail, then load again and
fail again...
So the number of iTLB loads is larger than misses.
Maybe it's related to high nmi rate, where the nmi handler clear my iTLB?
Let me try to remove the nmi interference first.
>> Do you know any way to improve iTLB hit rate?
>
> The xdpsock code should be small enough to fit in the iCache, but it
> might be layout in memory in an unfortunate way. You could play with
> rearranging the C-code (look at the objdump alignments).
>
> If you want to know the details about code alignment issue, and how to
> troubleshoot them, you should read this VERY excellent blog post by
> Denis Bakhvalov:
> https://dendibakh.github.io/blog/2018/01/18/Code_alignment_issues
Thanks for the link.
William
^ permalink raw reply
* Re: [PATCH V5 net-next 06/14] net/tls: Add generic NIC offload infrastructure
From: Kirill Tkhai @ 2018-03-28 15:11 UTC (permalink / raw)
To: Saeed Mahameed, David S. Miller
Cc: netdev, Dave Watson, Boris Pismenny, Ilya Lesokhin,
Aviad Yehezkel
In-Reply-To: <20180327235619.31308-7-saeedm@mellanox.com>
On 28.03.2018 02:56, Saeed Mahameed wrote:
> From: Ilya Lesokhin <ilyal@mellanox.com>
>
> This patch adds a generic infrastructure to offload TLS crypto to a
> network device. It enables the kernel TLS socket to skip encryption
> and authentication operations on the transmit side of the data path.
> Leaving those computationally expensive operations to the NIC.
>
> The NIC offload infrastructure builds TLS records and pushes them to
> the TCP layer just like the SW KTLS implementation and using the same API.
> TCP segmentation is mostly unaffected. Currently the only exception is
> that we prevent mixed SKBs where only part of the payload requires
> offload. In the future we are likely to add a similar restriction
> following a change cipher spec record.
>
> The notable differences between SW KTLS and NIC offloaded TLS
> implementations are as follows:
> 1. The offloaded implementation builds "plaintext TLS record", those
> records contain plaintext instead of ciphertext and place holder bytes
> instead of authentication tags.
> 2. The offloaded implementation maintains a mapping from TCP sequence
> number to TLS records. Thus given a TCP SKB sent from a NIC offloaded
> TLS socket, we can use the tls NIC offload infrastructure to obtain
> enough context to encrypt the payload of the SKB.
> A TLS record is released when the last byte of the record is ack'ed,
> this is done through the new icsk_clean_acked callback.
>
> The infrastructure should be extendable to support various NIC offload
> implementations. However it is currently written with the
> implementation below in mind:
> The NIC assumes that packets from each offloaded stream are sent as
> plaintext and in-order. It keeps track of the TLS records in the TCP
> stream. When a packet marked for offload is transmitted, the NIC
> encrypts the payload in-place and puts authentication tags in the
> relevant place holders.
>
> The responsibility for handling out-of-order packets (i.e. TCP
> retransmission, qdisc drops) falls on the netdev driver.
>
> The netdev driver keeps track of the expected TCP SN from the NIC's
> perspective. If the next packet to transmit matches the expected TCP
> SN, the driver advances the expected TCP SN, and transmits the packet
> with TLS offload indication.
>
> If the next packet to transmit does not match the expected TCP SN. The
> driver calls the TLS layer to obtain the TLS record that includes the
> TCP of the packet for transmission. Using this TLS record, the driver
> posts a work entry on the transmit queue to reconstruct the NIC TLS
> state required for the offload of the out-of-order packet. It updates
> the expected TCP SN accordingly and transmits the now in-order packet.
> The same queue is used for packet transmission and TLS context
> reconstruction to avoid the need for flushing the transmit queue before
> issuing the context reconstruction request.
>
> Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> include/net/tls.h | 120 +++++--
> net/tls/Kconfig | 10 +
> net/tls/Makefile | 2 +
> net/tls/tls_device.c | 759 ++++++++++++++++++++++++++++++++++++++++++
> net/tls/tls_device_fallback.c | 454 +++++++++++++++++++++++++
> net/tls/tls_main.c | 120 ++++---
> net/tls/tls_sw.c | 132 ++++----
> 7 files changed, 1476 insertions(+), 121 deletions(-)
> create mode 100644 net/tls/tls_device.c
> create mode 100644 net/tls/tls_device_fallback.c
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 437a746300bf..0a8529e9ec21 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -57,21 +57,10 @@
>
> #define TLS_AAD_SPACE_SIZE 13
>
> -struct tls_sw_context {
> +struct tls_sw_context_tx {
What the reason splitting this into tx + rx does not go in separate patch?
> struct crypto_aead *aead_send;
> - struct crypto_aead *aead_recv;
> struct crypto_wait async_wait;
>
> - /* Receive context */
> - struct strparser strp;
> - void (*saved_data_ready)(struct sock *sk);
> - unsigned int (*sk_poll)(struct file *file, struct socket *sock,
> - struct poll_table_struct *wait);
> - struct sk_buff *recv_pkt;
> - u8 control;
> - bool decrypted;
> -
> - /* Sending context */
> char aad_space[TLS_AAD_SPACE_SIZE];
>
> unsigned int sg_plaintext_size;
> @@ -88,6 +77,50 @@ struct tls_sw_context {
> struct scatterlist sg_aead_out[2];
> };
>
> +struct tls_sw_context_rx {
> + struct crypto_aead *aead_recv;
> + struct crypto_wait async_wait;
> +
> + struct strparser strp;
> + void (*saved_data_ready)(struct sock *sk);
> + unsigned int (*sk_poll)(struct file *file, struct socket *sock,
> + struct poll_table_struct *wait);
> + struct sk_buff *recv_pkt;
> + u8 control;
> + bool decrypted;
> +};
> +
> +struct tls_record_info {
> + struct list_head list;
> + u32 end_seq;
> + int len;
> + int num_frags;
> + skb_frag_t frags[MAX_SKB_FRAGS];
> +};
> +
> +struct tls_offload_context {
> + struct crypto_aead *aead_send;
> + spinlock_t lock; /* protects records list */
> + struct list_head records_list;
> + struct tls_record_info *open_record;
> + struct tls_record_info *retransmit_hint;
> + u64 hint_record_sn;
> + u64 unacked_record_sn;
> +
> + struct scatterlist sg_tx_data[MAX_SKB_FRAGS];
> + void (*sk_destruct)(struct sock *sk);
> + u8 driver_state[];
> + /* The TLS layer reserves room for driver specific state
> + * Currently the belief is that there is not enough
> + * driver specific state to justify another layer of indirection
> + */
> +#define TLS_DRIVER_STATE_SIZE (max_t(size_t, 8, sizeof(void *)))
> +};
> +
> +#define TLS_OFFLOAD_CONTEXT_SIZE \
> + (ALIGN(sizeof(struct tls_offload_context), sizeof(void *)) + \
> + TLS_DRIVER_STATE_SIZE)
> +
> enum {
> TLS_PENDING_CLOSED_RECORD
> };
> @@ -112,9 +145,15 @@ struct tls_context {
> struct tls12_crypto_info_aes_gcm_128 crypto_recv_aes_gcm_128;
> };
>
> - void *priv_ctx;
> + struct list_head list;
> + struct net_device *netdev;
> + refcount_t refcount;
> +
> + void *priv_ctx_tx;
> + void *priv_ctx_rx;
>
> - u8 conf:2;
> + u8 tx_conf:2;
> + u8 rx_conf:2;
>
> struct cipher_context tx;
> struct cipher_context rx;
> @@ -149,7 +188,8 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
> int tls_sw_sendpage(struct sock *sk, struct page *page,
> int offset, size_t size, int flags);
> void tls_sw_close(struct sock *sk, long timeout);
> -void tls_sw_free_resources(struct sock *sk);
> +void tls_sw_free_resources_tx(struct sock *sk);
> +void tls_sw_free_resources_rx(struct sock *sk);
> int tls_sw_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
> int nonblock, int flags, int *addr_len);
> unsigned int tls_sw_poll(struct file *file, struct socket *sock,
> @@ -158,9 +198,28 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos,
> struct pipe_inode_info *pipe,
> size_t len, unsigned int flags);
>
> -void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
> -void tls_icsk_clean_acked(struct sock *sk);
You unexport this function, and it becomes static after this patch.
Why this cleanup can't go in separate patch?
> +int tls_set_device_offload(struct sock *sk, struct tls_context *ctx);
> +int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
> +int tls_device_sendpage(struct sock *sk, struct page *page,
> + int offset, size_t size, int flags);
> +void tls_device_sk_destruct(struct sock *sk);
> +void tls_device_init(void);
> +void tls_device_cleanup(void);
>
> +struct tls_record_info *tls_get_record(struct tls_offload_context *context,
> + u32 seq, u64 *p_record_sn);
> +
> +static inline bool tls_record_is_start_marker(struct tls_record_info *rec)
> +{
> + return rec->len == 0;
> +}
> +
> +static inline u32 tls_record_start_seq(struct tls_record_info *rec)
> +{
> + return rec->end_seq - rec->len;
> +}
> +
> +void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
> int tls_push_sg(struct sock *sk, struct tls_context *ctx,
> struct scatterlist *sg, u16 first_offset,
> int flags);
> @@ -197,6 +256,13 @@ static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
> return tls_ctx->pending_open_record_frags;
> }
>
> +static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk)
> +{
> + return sk_fullsock(sk) &&
> + /* matches smp_store_release in tls_set_device_offload */
> + smp_load_acquire(&sk->sk_destruct) == &tls_device_sk_destruct;
> +}
> +
> static inline void tls_err_abort(struct sock *sk, int err)
> {
> sk->sk_err = err;
> @@ -269,19 +335,33 @@ static inline struct tls_context *tls_get_ctx(const struct sock *sk)
> return icsk->icsk_ulp_data;
> }
>
> -static inline struct tls_sw_context *tls_sw_ctx(
> +static inline struct tls_sw_context_rx *tls_sw_ctx_rx(
> const struct tls_context *tls_ctx)
> {
> - return (struct tls_sw_context *)tls_ctx->priv_ctx;
> + return (struct tls_sw_context_rx *)tls_ctx->priv_ctx_rx;
This is still splitting in tx + tx. Why can't this go in separate patch?
> +}
> +
> +static inline struct tls_sw_context_tx *tls_sw_ctx_tx(
> + const struct tls_context *tls_ctx)
> +{
> + return (struct tls_sw_context_tx *)tls_ctx->priv_ctx_tx;
> }
>
> static inline struct tls_offload_context *tls_offload_ctx(
> const struct tls_context *tls_ctx)
> {
> - return (struct tls_offload_context *)tls_ctx->priv_ctx;
> + return (struct tls_offload_context *)tls_ctx->priv_ctx_tx;
> }
>
> int tls_proccess_cmsg(struct sock *sk, struct msghdr *msg,
> unsigned char *record_type);
>
> +struct sk_buff *tls_validate_xmit_skb(struct sock *sk,
> + struct net_device *dev,
> + struct sk_buff *skb);
> +
> +int tls_sw_fallback_init(struct sock *sk,
> + struct tls_offload_context *offload_ctx,
> + struct tls_crypto_info *crypto_info);
> +
> #endif /* _TLS_OFFLOAD_H */
> diff --git a/net/tls/Kconfig b/net/tls/Kconfig
> index 89b8745a986f..7ad4ded9d7ac 100644
> --- a/net/tls/Kconfig
> +++ b/net/tls/Kconfig
> @@ -14,3 +14,13 @@ config TLS
> encryption handling of the TLS protocol to be done in-kernel.
>
> If unsure, say N.
> +
> +config TLS_DEVICE
> + bool "Transport Layer Security HW offload"
> + depends on TLS
> + select SOCK_VALIDATE_XMIT
> + default n
> + ---help---
> + Enable kernel support for HW offload of the TLS protocol.
> +
> + If unsure, say N.
> diff --git a/net/tls/Makefile b/net/tls/Makefile
> index a930fd1c4f7b..4d6b728a67d0 100644
> --- a/net/tls/Makefile
> +++ b/net/tls/Makefile
> @@ -5,3 +5,5 @@
> obj-$(CONFIG_TLS) += tls.o
>
> tls-y := tls_main.o tls_sw.o
> +
> +tls-$(CONFIG_TLS_DEVICE) += tls_device.o tls_device_fallback.o
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> new file mode 100644
> index 000000000000..f33cd65efa8a
> --- /dev/null
> +++ b/net/tls/tls_device.c
> @@ -0,0 +1,759 @@
> +/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * - Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer.
> + *
> + * - Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/module.h>
> +#include <net/tcp.h>
> +#include <net/inet_common.h>
> +#include <linux/highmem.h>
> +#include <linux/netdevice.h>
> +
> +#include <net/tls.h>
> +#include <crypto/aead.h>
> +
> +/* device_offload_lock is used to synchronize tls_dev_add
> + * against NETDEV_DOWN notifications.
> + */
> +static DECLARE_RWSEM(device_offload_lock);
> +
> +static void tls_device_gc_task(struct work_struct *work);
> +
> +static DECLARE_WORK(tls_device_gc_work, tls_device_gc_task);
> +static LIST_HEAD(tls_device_gc_list);
> +static LIST_HEAD(tls_device_list);
> +static DEFINE_SPINLOCK(tls_device_lock);
> +
> +static void tls_device_free_ctx(struct tls_context *ctx)
> +{
> + struct tls_offload_context *offload_ctx = tls_offload_ctx(ctx);
> +
> + kfree(offload_ctx);
> + kfree(ctx);
> +}
> +
> +static void tls_device_gc_task(struct work_struct *work)
> +{
> + struct tls_context *ctx, *tmp;
> + unsigned long flags;
> + LIST_HEAD(gc_list);
> +
> + spin_lock_irqsave(&tls_device_lock, flags);
> + list_splice_init(&tls_device_gc_list, &gc_list);
> + spin_unlock_irqrestore(&tls_device_lock, flags);
> +
> + list_for_each_entry_safe(ctx, tmp, &gc_list, list) {
> + struct net_device *netdev = ctx->netdev;
> +
> + if (netdev) {
> + netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
> + TLS_OFFLOAD_CTX_DIR_TX);
> + dev_put(netdev);
> + }
> +
> + list_del(&ctx->list);
> + tls_device_free_ctx(ctx);
> + }
> +}
> +
> +static void tls_device_queue_ctx_destruction(struct tls_context *ctx)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&tls_device_lock, flags);
> + list_move_tail(&ctx->list, &tls_device_gc_list);
> +
> + /* schedule_work inside the spinlock
> + * to make sure tls_device_down waits for that work.
> + */
> + schedule_work(&tls_device_gc_work);
> +
> + spin_unlock_irqrestore(&tls_device_lock, flags);
> +}
> +
> +/* We assume that the socket is already connected */
> +static struct net_device *get_netdev_for_sock(struct sock *sk)
> +{
> + struct inet_sock *inet = inet_sk(sk);
> + struct net_device *netdev = NULL;
> +
> + netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);
> +
> + return netdev;
> +}
> +
> +static void destroy_record(struct tls_record_info *record)
> +{
> + int nr_frags = record->num_frags;
> + skb_frag_t *frag;
> +
> + while (nr_frags-- > 0) {
> + frag = &record->frags[nr_frags];
> + __skb_frag_unref(frag);
> + }
> + kfree(record);
> +}
> +
> +static void delete_all_records(struct tls_offload_context *offload_ctx)
> +{
> + struct tls_record_info *info, *temp;
> +
> + list_for_each_entry_safe(info, temp, &offload_ctx->records_list, list) {
> + list_del(&info->list);
> + destroy_record(info);
> + }
> +
> + offload_ctx->retransmit_hint = NULL;
> +}
> +
> +static void tls_icsk_clean_acked(struct sock *sk, u32 acked_seq)
> +{
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_record_info *info, *temp;
> + struct tls_offload_context *ctx;
> + u64 deleted_records = 0;
> + unsigned long flags;
> +
> + if (!tls_ctx)
> + return;
> +
> + ctx = tls_offload_ctx(tls_ctx);
> +
> + spin_lock_irqsave(&ctx->lock, flags);
> + info = ctx->retransmit_hint;
> + if (info && !before(acked_seq, info->end_seq)) {
> + ctx->retransmit_hint = NULL;
> + list_del(&info->list);
> + destroy_record(info);
> + deleted_records++;
> + }
> +
> + list_for_each_entry_safe(info, temp, &ctx->records_list, list) {
> + if (before(acked_seq, info->end_seq))
> + break;
> + list_del(&info->list);
> +
> + destroy_record(info);
> + deleted_records++;
> + }
> +
> + ctx->unacked_record_sn += deleted_records;
> + spin_unlock_irqrestore(&ctx->lock, flags);
> +}
> +
> +/* At this point, there should be no references on this
> + * socket and no in-flight SKBs associated with this
> + * socket, so it is safe to free all the resources.
> + */
> +void tls_device_sk_destruct(struct sock *sk)
> +{
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
> +
> + if (ctx->open_record)
> + destroy_record(ctx->open_record);
> +
> + delete_all_records(ctx);
> + crypto_free_aead(ctx->aead_send);
> + ctx->sk_destruct(sk);
> + clean_acked_data_disable(inet_csk(sk));
> +
> + if (refcount_dec_and_test(&tls_ctx->refcount))
> + tls_device_queue_ctx_destruction(tls_ctx);
> +}
> +EXPORT_SYMBOL(tls_device_sk_destruct);
> +
> +static void tls_append_frag(struct tls_record_info *record,
> + struct page_frag *pfrag,
> + int size)
> +{
> + skb_frag_t *frag;
> +
> + frag = &record->frags[record->num_frags - 1];
> + if (frag->page.p == pfrag->page &&
> + frag->page_offset + frag->size == pfrag->offset) {
> + frag->size += size;
> + } else {
> + ++frag;
> + frag->page.p = pfrag->page;
> + frag->page_offset = pfrag->offset;
> + frag->size = size;
> + ++record->num_frags;
> + get_page(pfrag->page);
> + }
> +
> + pfrag->offset += size;
> + record->len += size;
> +}
> +
> +static int tls_push_record(struct sock *sk,
> + struct tls_context *ctx,
> + struct tls_offload_context *offload_ctx,
> + struct tls_record_info *record,
> + struct page_frag *pfrag,
> + int flags,
> + unsigned char record_type)
> +{
> + struct tcp_sock *tp = tcp_sk(sk);
> + struct page_frag dummy_tag_frag;
> + skb_frag_t *frag;
> + int i;
> +
> + /* fill prepend */
> + frag = &record->frags[0];
> + tls_fill_prepend(ctx,
> + skb_frag_address(frag),
> + record->len - ctx->tx.prepend_size,
> + record_type);
> +
> + /* HW doesn't care about the data in the tag, because it fills it. */
> + dummy_tag_frag.page = skb_frag_page(frag);
> + dummy_tag_frag.offset = 0;
> +
> + tls_append_frag(record, &dummy_tag_frag, ctx->tx.tag_size);
> + record->end_seq = tp->write_seq + record->len;
> + spin_lock_irq(&offload_ctx->lock);
> + list_add_tail(&record->list, &offload_ctx->records_list);
> + spin_unlock_irq(&offload_ctx->lock);
> + offload_ctx->open_record = NULL;
> + set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
> + tls_advance_record_sn(sk, &ctx->tx);
> +
> + for (i = 0; i < record->num_frags; i++) {
> + frag = &record->frags[i];
> + sg_unmark_end(&offload_ctx->sg_tx_data[i]);
> + sg_set_page(&offload_ctx->sg_tx_data[i], skb_frag_page(frag),
> + frag->size, frag->page_offset);
> + sk_mem_charge(sk, frag->size);
> + get_page(skb_frag_page(frag));
> + }
> + sg_mark_end(&offload_ctx->sg_tx_data[record->num_frags - 1]);
> +
> + /* all ready, send */
> + return tls_push_sg(sk, ctx, offload_ctx->sg_tx_data, 0, flags);
> +}
> +
> +static int tls_create_new_record(struct tls_offload_context *offload_ctx,
> + struct page_frag *pfrag,
> + size_t prepend_size)
> +{
> + struct tls_record_info *record;
> + skb_frag_t *frag;
> +
> + record = kmalloc(sizeof(*record), GFP_KERNEL);
> + if (!record)
> + return -ENOMEM;
> +
> + frag = &record->frags[0];
> + __skb_frag_set_page(frag, pfrag->page);
> + frag->page_offset = pfrag->offset;
> + skb_frag_size_set(frag, prepend_size);
> +
> + get_page(pfrag->page);
> + pfrag->offset += prepend_size;
> +
> + record->num_frags = 1;
> + record->len = prepend_size;
> + offload_ctx->open_record = record;
> + return 0;
> +}
> +
> +static int tls_do_allocation(struct sock *sk,
> + struct tls_offload_context *offload_ctx,
> + struct page_frag *pfrag,
> + size_t prepend_size)
> +{
> + int ret;
> +
> + if (!offload_ctx->open_record) {
> + if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
> + sk->sk_allocation))) {
> + sk->sk_prot->enter_memory_pressure(sk);
> + sk_stream_moderate_sndbuf(sk);
> + return -ENOMEM;
> + }
> +
> + ret = tls_create_new_record(offload_ctx, pfrag, prepend_size);
> + if (ret)
> + return ret;
> +
> + if (pfrag->size > pfrag->offset)
> + return 0;
> + }
> +
> + if (!sk_page_frag_refill(sk, pfrag))
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int tls_push_data(struct sock *sk,
> + struct iov_iter *msg_iter,
> + size_t size, int flags,
> + unsigned char record_type)
> +{
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
> + int tls_push_record_flags = flags | MSG_SENDPAGE_NOTLAST;
> + int more = flags & (MSG_SENDPAGE_NOTLAST | MSG_MORE);
> + struct tls_record_info *record = ctx->open_record;
> + struct page_frag *pfrag;
> + size_t orig_size = size;
> + u32 max_open_record_len;
> + int copy, rc = 0;
> + bool done = false;
> + long timeo;
> +
> + if (flags &
> + ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL | MSG_SENDPAGE_NOTLAST))
> + return -ENOTSUPP;
> +
> + if (sk->sk_err)
> + return -sk->sk_err;
> +
> + timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
> + rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
> + if (rc < 0)
> + return rc;
> +
> + pfrag = sk_page_frag(sk);
> +
> + /* TLS_HEADER_SIZE is not counted as part of the TLS record, and
> + * we need to leave room for an authentication tag.
> + */
> + max_open_record_len = TLS_MAX_PAYLOAD_SIZE +
> + tls_ctx->tx.prepend_size;
> + do {
> + rc = tls_do_allocation(sk, ctx, pfrag,
> + tls_ctx->tx.prepend_size);
> + if (rc) {
> + rc = sk_stream_wait_memory(sk, &timeo);
> + if (!rc)
> + continue;
> +
> + record = ctx->open_record;
> + if (!record)
> + break;
> +handle_error:
> + if (record_type != TLS_RECORD_TYPE_DATA) {
> + /* avoid sending partial
> + * record with type !=
> + * application_data
> + */
> + size = orig_size;
> + destroy_record(record);
> + ctx->open_record = NULL;
> + } else if (record->len > tls_ctx->tx.prepend_size) {
> + goto last_record;
> + }
> +
> + break;
> + }
> +
> + record = ctx->open_record;
> + copy = min_t(size_t, size, (pfrag->size - pfrag->offset));
> + copy = min_t(size_t, copy, (max_open_record_len - record->len));
> +
> + if (copy_from_iter_nocache(page_address(pfrag->page) +
> + pfrag->offset,
> + copy, msg_iter) != copy) {
> + rc = -EFAULT;
> + goto handle_error;
> + }
> + tls_append_frag(record, pfrag, copy);
> +
> + size -= copy;
> + if (!size) {
> +last_record:
> + tls_push_record_flags = flags;
> + if (more) {
> + tls_ctx->pending_open_record_frags =
> + record->num_frags;
> + break;
> + }
> +
> + done = true;
> + }
> +
> + if (done || record->len >= max_open_record_len ||
> + (record->num_frags >= MAX_SKB_FRAGS - 1)) {
> + rc = tls_push_record(sk,
> + tls_ctx,
> + ctx,
> + record,
> + pfrag,
> + tls_push_record_flags,
> + record_type);
> + if (rc < 0)
> + break;
> + }
> + } while (!done);
> +
> + if (orig_size - size > 0)
> + rc = orig_size - size;
> +
> + return rc;
> +}
> +
> +int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> +{
> + unsigned char record_type = TLS_RECORD_TYPE_DATA;
> + int rc;
> +
> + lock_sock(sk);
> +
> + if (unlikely(msg->msg_controllen)) {
> + rc = tls_proccess_cmsg(sk, msg, &record_type);
> + if (rc)
> + goto out;
> + }
> +
> + rc = tls_push_data(sk, &msg->msg_iter, size,
> + msg->msg_flags, record_type);
> +
> +out:
> + release_sock(sk);
> + return rc;
> +}
> +
> +int tls_device_sendpage(struct sock *sk, struct page *page,
> + int offset, size_t size, int flags)
> +{
> + struct iov_iter msg_iter;
> + char *kaddr = kmap(page);
> + struct kvec iov;
> + int rc;
> +
> + if (flags & MSG_SENDPAGE_NOTLAST)
> + flags |= MSG_MORE;
> +
> + lock_sock(sk);
> +
> + if (flags & MSG_OOB) {
> + rc = -ENOTSUPP;
> + goto out;
> + }
> +
> + iov.iov_base = kaddr + offset;
> + iov.iov_len = size;
> + iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, &iov, 1, size);
> + rc = tls_push_data(sk, &msg_iter, size,
> + flags, TLS_RECORD_TYPE_DATA);
> + kunmap(page);
> +
> +out:
> + release_sock(sk);
> + return rc;
> +}
> +
> +struct tls_record_info *tls_get_record(struct tls_offload_context *context,
> + u32 seq, u64 *p_record_sn)
> +{
> + u64 record_sn = context->hint_record_sn;
> + struct tls_record_info *info;
> +
> + info = context->retransmit_hint;
> + if (!info ||
> + before(seq, info->end_seq - info->len)) {
> + /* if retransmit_hint is irrelevant start
> + * from the beggining of the list
> + */
> + info = list_first_entry(&context->records_list,
> + struct tls_record_info, list);
> + record_sn = context->unacked_record_sn;
> + }
> +
> + list_for_each_entry_from(info, &context->records_list, list) {
> + if (before(seq, info->end_seq)) {
> + if (!context->retransmit_hint ||
> + after(info->end_seq,
> + context->retransmit_hint->end_seq)) {
> + context->hint_record_sn = record_sn;
> + context->retransmit_hint = info;
> + }
> + *p_record_sn = record_sn;
> + return info;
> + }
> + record_sn++;
> + }
> +
> + return NULL;
> +}
> +EXPORT_SYMBOL(tls_get_record);
> +
> +static int tls_device_push_pending_record(struct sock *sk, int flags)
> +{
> + struct iov_iter msg_iter;
> +
> + iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, NULL, 0, 0);
> + return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
> +}
> +
> +int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
> +{
> + u16 nonce_size, tag_size, iv_size, rec_seq_size;
> + struct tls_record_info *start_marker_record;
> + struct tls_offload_context *offload_ctx;
> + struct tls_crypto_info *crypto_info;
> + struct net_device *netdev;
> + char *iv, *rec_seq;
> + struct sk_buff *skb;
> + int rc = -EINVAL;
> + __be64 rcd_sn;
> +
> + if (!ctx)
> + goto out;
> +
> + if (ctx->priv_ctx_tx) {
> + rc = -EEXIST;
> + goto out;
> + }
> +
> + start_marker_record = kmalloc(sizeof(*start_marker_record), GFP_KERNEL);
> + if (!start_marker_record) {
> + rc = -ENOMEM;
> + goto out;
> + }
> +
> + offload_ctx = kzalloc(TLS_OFFLOAD_CONTEXT_SIZE, GFP_KERNEL);
> + if (!offload_ctx) {
> + rc = -ENOMEM;
> + goto free_marker_record;
> + }
> +
> + crypto_info = &ctx->crypto_send;
> + switch (crypto_info->cipher_type) {
> + case TLS_CIPHER_AES_GCM_128:
> + nonce_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
> + tag_size = TLS_CIPHER_AES_GCM_128_TAG_SIZE;
> + iv_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
> + iv = ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->iv;
> + rec_seq_size = TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE;
> + rec_seq =
> + ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->rec_seq;
> + break;
> + default:
> + rc = -EINVAL;
> + goto free_offload_ctx;
> + }
> +
> + ctx->tx.prepend_size = TLS_HEADER_SIZE + nonce_size;
> + ctx->tx.tag_size = tag_size;
> + ctx->tx.overhead_size = ctx->tx.prepend_size + ctx->tx.tag_size;
> + ctx->tx.iv_size = iv_size;
> + ctx->tx.iv = kmalloc(iv_size + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
> + GFP_KERNEL);
> + if (!ctx->tx.iv) {
> + rc = -ENOMEM;
> + goto free_offload_ctx;
> + }
> +
> + memcpy(ctx->tx.iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv, iv_size);
> +
> + ctx->tx.rec_seq_size = rec_seq_size;
> + ctx->tx.rec_seq = kmalloc(rec_seq_size, GFP_KERNEL);
> + if (!ctx->tx.rec_seq) {
> + rc = -ENOMEM;
> + goto free_iv;
> + }
> + memcpy(ctx->tx.rec_seq, rec_seq, rec_seq_size);
> +
> + rc = tls_sw_fallback_init(sk, offload_ctx, crypto_info);
> + if (rc)
> + goto free_rec_seq;
> +
> + /* start at rec_seq - 1 to account for the start marker record */
> + memcpy(&rcd_sn, ctx->tx.rec_seq, sizeof(rcd_sn));
> + offload_ctx->unacked_record_sn = be64_to_cpu(rcd_sn) - 1;
> +
> + start_marker_record->end_seq = tcp_sk(sk)->write_seq;
> + start_marker_record->len = 0;
> + start_marker_record->num_frags = 0;
> +
> + INIT_LIST_HEAD(&offload_ctx->records_list);
> + list_add_tail(&start_marker_record->list, &offload_ctx->records_list);
> + spin_lock_init(&offload_ctx->lock);
> +
> + clean_acked_data_enable(inet_csk(sk), &tls_icsk_clean_acked);
> + ctx->push_pending_record = tls_device_push_pending_record;
> + offload_ctx->sk_destruct = sk->sk_destruct;
> +
> + /* TLS offload is greatly simplified if we don't send
> + * SKBs where only part of the payload needs to be encrypted.
> + * So mark the last skb in the write queue as end of record.
> + */
> + skb = tcp_write_queue_tail(sk);
> + if (skb)
> + TCP_SKB_CB(skb)->eor = 1;
> +
> + refcount_set(&ctx->refcount, 1);
> +
> + /* We support starting offload on multiple sockets
> + * concurrently, so we only need a read lock here.
> + * This lock must preceed get_netdev_for_sock to prevent races between
> + * NETDEV_DOWN and setsockopt.
> + */
> + down_read(&device_offload_lock);
> + netdev = get_netdev_for_sock(sk);
> + if (!netdev) {
> + pr_err_ratelimited("%s: netdev not found\n", __func__);
> + rc = -EINVAL;
> + goto release_lock;
> + }
> +
> + if (!(netdev->features & NETIF_F_HW_TLS_TX)) {
> + rc = -ENOTSUPP;
> + goto release_netdev;
> + }
> +
> + /* Avoid offloading if the device is down
> + * We don't want to offload new flows after
> + * the NETDEV_DOWN event
> + */
> + if (!(netdev->flags & IFF_UP)) {
> + rc = -EINVAL;
> + goto release_netdev;
> + }
> +
> + ctx->priv_ctx_tx = offload_ctx;
> + rc = netdev->tlsdev_ops->tls_dev_add(netdev, sk, TLS_OFFLOAD_CTX_DIR_TX,
> + &ctx->crypto_send,
> + tcp_sk(sk)->write_seq);
> + if (rc)
> + goto release_netdev;
> +
> + ctx->netdev = netdev;
> +
> + spin_lock_irq(&tls_device_lock);
> + list_add_tail(&ctx->list, &tls_device_list);
> + spin_unlock_irq(&tls_device_lock);
> +
> + sk->sk_validate_xmit_skb = tls_validate_xmit_skb;
> + /* following this assignment tls_is_sk_tx_device_offloaded
> + * will return true and the context might be accessed
> + * by the netdev's xmit function.
> + */
> + smp_store_release(&sk->sk_destruct,
> + &tls_device_sk_destruct);
> + up_read(&device_offload_lock);
> + goto out;
> +
> +release_netdev:
> + dev_put(netdev);
> +release_lock:
> + up_read(&device_offload_lock);
> + clean_acked_data_disable(inet_csk(sk));
> + crypto_free_aead(offload_ctx->aead_send);
> +free_rec_seq:
> + kfree(ctx->tx.rec_seq);
> +free_iv:
> + kfree(ctx->tx.iv);
> +free_offload_ctx:
> + kfree(offload_ctx);
> + ctx->priv_ctx_tx = NULL;
> +free_marker_record:
> + kfree(start_marker_record);
> +out:
> + return rc;
> +}
> +
> +static int tls_device_down(struct net_device *netdev)
> +{
> + struct tls_context *ctx, *tmp;
> + unsigned long flags;
> + LIST_HEAD(list);
> +
> + /* Request a write lock to block new offload attempts */
> + down_write(&device_offload_lock);
> +
> + spin_lock_irqsave(&tls_device_lock, flags);
> + list_for_each_entry_safe(ctx, tmp, &tls_device_list, list) {
> + if (ctx->netdev != netdev ||
> + !refcount_inc_not_zero(&ctx->refcount))
> + continue;
> +
> + list_move(&ctx->list, &list);
> + }
> + spin_unlock_irqrestore(&tls_device_lock, flags);
> +
> + list_for_each_entry_safe(ctx, tmp, &list, list) {
> + netdev->tlsdev_ops->tls_dev_del(netdev, ctx,
> + TLS_OFFLOAD_CTX_DIR_TX);
> + ctx->netdev = NULL;
> + dev_put(netdev);
> + list_del_init(&ctx->list);
> +
> + if (refcount_dec_and_test(&ctx->refcount))
> + tls_device_free_ctx(ctx);
> + }
> +
> + up_write(&device_offload_lock);
> +
> + flush_work(&tls_device_gc_work);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static int tls_dev_event(struct notifier_block *this, unsigned long event,
> + void *ptr)
> +{
> + struct net_device *dev = netdev_notifier_info_to_dev(ptr);
> +
> + if (!(dev->features & NETIF_F_HW_TLS_TX))
> + return NOTIFY_DONE;
> +
> + switch (event) {
> + case NETDEV_REGISTER:
> + case NETDEV_FEAT_CHANGE:
> + if (dev->tlsdev_ops &&
> + dev->tlsdev_ops->tls_dev_add &&
> + dev->tlsdev_ops->tls_dev_del)
> + return NOTIFY_DONE;
> + else
> + return NOTIFY_BAD;
> + case NETDEV_DOWN:
> + return tls_device_down(dev);
> + }
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block tls_dev_notifier = {
> + .notifier_call = tls_dev_event,
> +};
> +
> +void __init tls_device_init(void)
> +{
> + register_netdevice_notifier(&tls_dev_notifier);
> +}
> +
> +void __exit tls_device_cleanup(void)
> +{
> + unregister_netdevice_notifier(&tls_dev_notifier);
> + flush_work(&tls_device_gc_work);
> +}
> diff --git a/net/tls/tls_device_fallback.c b/net/tls/tls_device_fallback.c
> new file mode 100644
> index 000000000000..4e0f3d56b6dd
> --- /dev/null
> +++ b/net/tls/tls_device_fallback.c
> @@ -0,0 +1,454 @@
> +/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
> + *
> + * This software is available to you under a choice of one of two
> + * licenses. You may choose to be licensed under the terms of the GNU
> + * General Public License (GPL) Version 2, available from the file
> + * COPYING in the main directory of this source tree, or the
> + * OpenIB.org BSD license below:
> + *
> + * Redistribution and use in source and binary forms, with or
> + * without modification, are permitted provided that the following
> + * conditions are met:
> + *
> + * - Redistributions of source code must retain the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer.
> + *
> + * - Redistributions in binary form must reproduce the above
> + * copyright notice, this list of conditions and the following
> + * disclaimer in the documentation and/or other materials
> + * provided with the distribution.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <net/tls.h>
> +#include <crypto/aead.h>
> +#include <crypto/scatterwalk.h>
> +#include <net/ip6_checksum.h>
> +
> +static void chain_to_walk(struct scatterlist *sg, struct scatter_walk *walk)
> +{
> + struct scatterlist *src = walk->sg;
> + int diff = walk->offset - src->offset;
> +
> + sg_set_page(sg, sg_page(src),
> + src->length - diff, walk->offset);
> +
> + scatterwalk_crypto_chain(sg, sg_next(src), 0, 2);
> +}
> +
> +static int tls_enc_record(struct aead_request *aead_req,
> + struct crypto_aead *aead, char *aad,
> + char *iv, __be64 rcd_sn,
> + struct scatter_walk *in,
> + struct scatter_walk *out, int *in_len)
> +{
> + unsigned char buf[TLS_HEADER_SIZE + TLS_CIPHER_AES_GCM_128_IV_SIZE];
> + struct scatterlist sg_in[3];
> + struct scatterlist sg_out[3];
> + u16 len;
> + int rc;
> +
> + len = min_t(int, *in_len, ARRAY_SIZE(buf));
> +
> + scatterwalk_copychunks(buf, in, len, 0);
> + scatterwalk_copychunks(buf, out, len, 1);
> +
> + *in_len -= len;
> + if (!*in_len)
> + return 0;
> +
> + scatterwalk_pagedone(in, 0, 1);
> + scatterwalk_pagedone(out, 1, 1);
> +
> + len = buf[4] | (buf[3] << 8);
> + len -= TLS_CIPHER_AES_GCM_128_IV_SIZE;
> +
> + tls_make_aad(aad, len - TLS_CIPHER_AES_GCM_128_TAG_SIZE,
> + (char *)&rcd_sn, sizeof(rcd_sn), buf[0]);
> +
> + memcpy(iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, buf + TLS_HEADER_SIZE,
> + TLS_CIPHER_AES_GCM_128_IV_SIZE);
> +
> + sg_init_table(sg_in, ARRAY_SIZE(sg_in));
> + sg_init_table(sg_out, ARRAY_SIZE(sg_out));
> + sg_set_buf(sg_in, aad, TLS_AAD_SPACE_SIZE);
> + sg_set_buf(sg_out, aad, TLS_AAD_SPACE_SIZE);
> + chain_to_walk(sg_in + 1, in);
> + chain_to_walk(sg_out + 1, out);
> +
> + *in_len -= len;
> + if (*in_len < 0) {
> + *in_len += TLS_CIPHER_AES_GCM_128_TAG_SIZE;
> + /* the input buffer doesn't contain the entire record.
> + * trim len accordingly. The resulting authentication tag
> + * will contain garbage, but we don't care, so we won't
> + * include any of it in the output skb
> + * Note that we assume the output buffer length
> + * is larger then input buffer length + tag size
> + */
> + if (*in_len < 0)
> + len += *in_len;
> +
> + *in_len = 0;
> + }
> +
> + if (*in_len) {
> + scatterwalk_copychunks(NULL, in, len, 2);
> + scatterwalk_pagedone(in, 0, 1);
> + scatterwalk_copychunks(NULL, out, len, 2);
> + scatterwalk_pagedone(out, 1, 1);
> + }
> +
> + len -= TLS_CIPHER_AES_GCM_128_TAG_SIZE;
> + aead_request_set_crypt(aead_req, sg_in, sg_out, len, iv);
> +
> + rc = crypto_aead_encrypt(aead_req);
> +
> + return rc;
> +}
> +
> +static void tls_init_aead_request(struct aead_request *aead_req,
> + struct crypto_aead *aead)
> +{
> + aead_request_set_tfm(aead_req, aead);
> + aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
> +}
> +
> +static struct aead_request *tls_alloc_aead_request(struct crypto_aead *aead,
> + gfp_t flags)
> +{
> + unsigned int req_size = sizeof(struct aead_request) +
> + crypto_aead_reqsize(aead);
> + struct aead_request *aead_req;
> +
> + aead_req = kzalloc(req_size, flags);
> + if (aead_req)
> + tls_init_aead_request(aead_req, aead);
> + return aead_req;
> +}
> +
> +static int tls_enc_records(struct aead_request *aead_req,
> + struct crypto_aead *aead, struct scatterlist *sg_in,
> + struct scatterlist *sg_out, char *aad, char *iv,
> + u64 rcd_sn, int len)
> +{
> + struct scatter_walk out, in;
> + int rc;
> +
> + scatterwalk_start(&in, sg_in);
> + scatterwalk_start(&out, sg_out);
> +
> + do {
> + rc = tls_enc_record(aead_req, aead, aad, iv,
> + cpu_to_be64(rcd_sn), &in, &out, &len);
> + rcd_sn++;
> +
> + } while (rc == 0 && len);
> +
> + scatterwalk_done(&in, 0, 0);
> + scatterwalk_done(&out, 1, 0);
> +
> + return rc;
> +}
> +
> +/* Can't use icsk->icsk_af_ops->send_check here because the ip addresses
> + * might have been changed by NAT.
> + */
> +static void update_chksum(struct sk_buff *skb, int headln)
> +{
> + struct tcphdr *th = tcp_hdr(skb);
> + int datalen = skb->len - headln;
> + const struct ipv6hdr *ipv6h;
> + const struct iphdr *iph;
> +
> + /* We only changed the payload so if we are using partial we don't
> + * need to update anything.
> + */
> + if (likely(skb->ip_summed == CHECKSUM_PARTIAL))
> + return;
> +
> + skb->ip_summed = CHECKSUM_PARTIAL;
> + skb->csum_start = skb_transport_header(skb) - skb->head;
> + skb->csum_offset = offsetof(struct tcphdr, check);
> +
> + if (skb->sk->sk_family == AF_INET6) {
> + ipv6h = ipv6_hdr(skb);
> + th->check = ~csum_ipv6_magic(&ipv6h->saddr, &ipv6h->daddr,
> + datalen, IPPROTO_TCP, 0);
> + } else {
> + iph = ip_hdr(skb);
> + th->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr, datalen,
> + IPPROTO_TCP, 0);
> + }
> +}
> +
> +static void complete_skb(struct sk_buff *nskb, struct sk_buff *skb, int headln)
> +{
> + skb_copy_header(nskb, skb);
> +
> + skb_put(nskb, skb->len);
> + memcpy(nskb->data, skb->data, headln);
> + update_chksum(nskb, headln);
> +
> + nskb->destructor = skb->destructor;
> + nskb->sk = skb->sk;
> + skb->destructor = NULL;
> + skb->sk = NULL;
> + refcount_add(nskb->truesize - skb->truesize,
> + &nskb->sk->sk_wmem_alloc);
> +}
> +
> +/* This function may be called after the user socket is already
> + * closed so make sure we don't use anything freed during
> + * tls_sk_proto_close here
> + */
> +
> +static int fill_sg_in(struct scatterlist *sg_in,
> + struct sk_buff *skb,
> + struct tls_offload_context *ctx,
> + u64 *rcd_sn,
> + s32 *sync_size,
> + int *resync_sgs)
> +{
> + int tcp_payload_offset = skb_transport_offset(skb) + tcp_hdrlen(skb);
> + int payload_len = skb->len - tcp_payload_offset;
> + u32 tcp_seq = ntohl(tcp_hdr(skb)->seq);
> + struct tls_record_info *record;
> + unsigned long flags;
> + int remaining;
> + int i;
> +
> + spin_lock_irqsave(&ctx->lock, flags);
> + record = tls_get_record(ctx, tcp_seq, rcd_sn);
> + if (!record) {
> + spin_unlock_irqrestore(&ctx->lock, flags);
> + WARN(1, "Record not found for seq %u\n", tcp_seq);
> + return -EINVAL;
> + }
> +
> + *sync_size = tcp_seq - tls_record_start_seq(record);
> + if (*sync_size < 0) {
> + int is_start_marker = tls_record_is_start_marker(record);
> +
> + spin_unlock_irqrestore(&ctx->lock, flags);
> + /* This should only occur if the relevant record was
> + * already acked. In that case it should be ok
> + * to drop the packet and avoid retransmission.
> + *
> + * There is a corner case where the packet contains
> + * both an acked and a non-acked record.
> + * We currently don't handle that case and rely
> + * on TCP to retranmit a packet that doesn't contain
> + * already acked payload.
> + */
> + if (!is_start_marker) {
> + *sync_size = 0;
> + }
> + pr_err("err_sync %d\n", *sync_size);
> + return -EINVAL;
> + }
> +
> + remaining = *sync_size;
> + for (i = 0; remaining > 0; i++) {
> + skb_frag_t *frag = &record->frags[i];
> +
> + __skb_frag_ref(frag);
> + sg_set_page(sg_in + i, skb_frag_page(frag),
> + skb_frag_size(frag), frag->page_offset);
> +
> + remaining -= skb_frag_size(frag);
> +
> + if (remaining < 0)
> + sg_in[i].length += remaining;
> + }
> + *resync_sgs = i;
> +
> + spin_unlock_irqrestore(&ctx->lock, flags);
> + if (skb_to_sgvec(skb, &sg_in[i], tcp_payload_offset, payload_len) < 0)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static void fill_sg_out(struct scatterlist sg_out[3], void *buf,
> + struct tls_context *tls_ctx,
> + struct sk_buff *nskb,
> + int tcp_payload_offset,
> + int payload_len,
> + int sync_size,
> + void *dummy_buf)
> +{
> +
> +
> + sg_set_buf(&sg_out[0], dummy_buf, sync_size);
> + sg_set_buf(&sg_out[1], nskb->data + tcp_payload_offset, payload_len);
> + /* Add room for authentication tag produced by crypto */
> + dummy_buf += sync_size;
> + sg_set_buf(&sg_out[2], dummy_buf, TLS_CIPHER_AES_GCM_128_TAG_SIZE);
> +}
> +
> +static struct sk_buff *tls_enc_skb(struct tls_context *tls_ctx,
> + struct scatterlist sg_out[3],
> + struct scatterlist *sg_in,
> + struct sk_buff *skb,
> + s32 sync_size, u64 rcd_sn)
> +{
> + int tcp_payload_offset = skb_transport_offset(skb) + tcp_hdrlen(skb);
> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
> + int payload_len = skb->len - tcp_payload_offset;
> + void *buf, *iv, *aad, *dummy_buf;
> + struct aead_request *aead_req;
> + struct sk_buff *nskb = NULL;
> + int buf_len;
> +
> + aead_req = tls_alloc_aead_request(ctx->aead_send, GFP_ATOMIC);
> + if (!aead_req)
> + return NULL;
> +
> + buf_len = TLS_CIPHER_AES_GCM_128_SALT_SIZE +
> + TLS_CIPHER_AES_GCM_128_IV_SIZE +
> + TLS_AAD_SPACE_SIZE +
> + sync_size +
> + TLS_CIPHER_AES_GCM_128_TAG_SIZE;
> + buf = kmalloc(buf_len, GFP_ATOMIC);
> + if (!buf)
> + goto free_req;
> +
> + iv = buf;
> + memcpy(iv, tls_ctx->crypto_send_aes_gcm_128.salt,
> + TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> + aad = buf + TLS_CIPHER_AES_GCM_128_SALT_SIZE +
> + TLS_CIPHER_AES_GCM_128_IV_SIZE;
> + dummy_buf = aad + TLS_AAD_SPACE_SIZE;
> +
> + nskb = alloc_skb(skb_headroom(skb) + skb->len, GFP_ATOMIC);
> + if (!nskb)
> + goto free_buf;
> +
> + skb_reserve(nskb, skb_headroom(skb));
> +
> + fill_sg_out(sg_out, buf, tls_ctx, nskb, tcp_payload_offset,
> + payload_len, sync_size, dummy_buf);
> +
> + if (tls_enc_records(aead_req, ctx->aead_send, sg_in, sg_out, aad, iv,
> + rcd_sn, sync_size + payload_len) < 0)
> + goto free_nskb;
> +
> + complete_skb(nskb, skb, tcp_payload_offset);
> +
> + /* validate_xmit_skb_list assumes that if the skb wasn't segmented
> + * nskb->prev will point to the skb itself
> + */
> + nskb->prev = nskb;
> +
> +free_buf:
> + kfree(buf);
> +free_req:
> + kfree(aead_req);
> + return nskb;
> +free_nskb:
> + kfree_skb(nskb);
> + nskb = NULL;
> + goto free_buf;
> +}
> +
> +static struct sk_buff *tls_sw_fallback(struct sock *sk, struct sk_buff *skb)
> +{
> + int tcp_payload_offset = skb_transport_offset(skb) + tcp_hdrlen(skb);
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
> + int payload_len = skb->len - tcp_payload_offset;
> + struct scatterlist *sg_in, sg_out[3];
> + struct sk_buff *nskb = NULL;
> + int sg_in_max_elements;
> + int resync_sgs = 0;
> + s32 sync_size = 0;
> + u64 rcd_sn;
> +
> + /* worst case is:
> + * MAX_SKB_FRAGS in tls_record_info
> + * MAX_SKB_FRAGS + 1 in SKB head and frags.
> + */
> + sg_in_max_elements = 2 * MAX_SKB_FRAGS + 1;
> +
> + if (!payload_len)
> + return skb;
> +
> + sg_in = kmalloc_array(sg_in_max_elements, sizeof(*sg_in), GFP_ATOMIC);
> + if (!sg_in)
> + goto free_orig;
> +
> + sg_init_table(sg_in, sg_in_max_elements);
> + sg_init_table(sg_out, ARRAY_SIZE(sg_out));
> +
> + if (fill_sg_in(sg_in, skb, ctx, &rcd_sn, &sync_size, &resync_sgs)) {
> + /* bypass packets before kernel TLS socket option was set */
> + if (sync_size < 0 && payload_len <= -sync_size)
> + nskb = skb_get(skb);
> + goto put_sg;
> + }
> +
> + nskb = tls_enc_skb(tls_ctx, sg_out, sg_in, skb, sync_size, rcd_sn);
> +
> +put_sg:
> + while (resync_sgs)
> + put_page(sg_page(&sg_in[--resync_sgs]));
> + kfree(sg_in);
> +free_orig:
> + kfree_skb(skb);
> + return nskb;
> +}
> +
> +struct sk_buff *tls_validate_xmit_skb(struct sock *sk,
> + struct net_device *dev,
> + struct sk_buff *skb)
> +{
> + if (dev == tls_get_ctx(sk)->netdev)
> + return skb;
> +
> + return tls_sw_fallback(sk, skb);
> +}
> +
> +int tls_sw_fallback_init(struct sock *sk,
> + struct tls_offload_context *offload_ctx,
> + struct tls_crypto_info *crypto_info)
> +{
> + const u8 *key;
> + int rc;
> +
> + offload_ctx->aead_send =
> + crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
> + if (IS_ERR(offload_ctx->aead_send)) {
> + rc = PTR_ERR(offload_ctx->aead_send);
> + pr_err_ratelimited("crypto_alloc_aead failed rc=%d\n", rc);
> + offload_ctx->aead_send = NULL;
> + goto err_out;
> + }
> +
> + key = ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->key;
> +
> + rc = crypto_aead_setkey(offload_ctx->aead_send, key,
> + TLS_CIPHER_AES_GCM_128_KEY_SIZE);
> + if (rc)
> + goto free_aead;
> +
> + rc = crypto_aead_setauthsize(offload_ctx->aead_send,
> + TLS_CIPHER_AES_GCM_128_TAG_SIZE);
> + if (rc)
> + goto free_aead;
> +
> + return 0;
> +free_aead:
> + crypto_free_aead(offload_ctx->aead_send);
> +err_out:
> + return rc;
> +}
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 6f5c1146da4a..47ed1b72b3ca 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -50,25 +50,25 @@ enum {
> TLSV6,
> TLS_NUM_PROTS,
> };
> -
> enum {
> TLS_BASE,
> - TLS_SW_TX,
> - TLS_SW_RX,
> - TLS_SW_RXTX,
> + TLS_SW,
> +#ifdef CONFIG_TLS_DEVICE
> + TLS_HW,
> +#endif
> TLS_NUM_CONFIG,
> };
>
> static struct proto *saved_tcpv6_prot;
> static DEFINE_MUTEX(tcpv6_prot_mutex);
> -static struct proto tls_prots[TLS_NUM_PROTS][TLS_NUM_CONFIG];
> +static struct proto tls_prots[TLS_NUM_PROTS][TLS_NUM_CONFIG][TLS_NUM_CONFIG];
> static struct proto_ops tls_sw_proto_ops;
>
> -static inline void update_sk_prot(struct sock *sk, struct tls_context *ctx)
> +static void update_sk_prot(struct sock *sk, struct tls_context *ctx)
All cleanups have to go in separate patches.
> {
> int ip_ver = sk->sk_family == AF_INET6 ? TLSV6 : TLSV4;
>
> - sk->sk_prot = &tls_prots[ip_ver][ctx->conf];
> + sk->sk_prot = &tls_prots[ip_ver][ctx->tx_conf][ctx->rx_conf];
> }
>
> int wait_on_pending_writer(struct sock *sk, long *timeo)
> @@ -241,7 +241,7 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
> lock_sock(sk);
> sk_proto_close = ctx->sk_proto_close;
>
> - if (ctx->conf == TLS_BASE) {
> + if (ctx->tx_conf == TLS_BASE && ctx->rx_conf == TLS_BASE) {
> kfree(ctx);
> goto skip_tx_cleanup;
> }
> @@ -262,17 +262,25 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
> }
> }
>
> - kfree(ctx->tx.rec_seq);
> - kfree(ctx->tx.iv);
> - kfree(ctx->rx.rec_seq);
> - kfree(ctx->rx.iv);
> + /* We need these for tls_sw_fallback handling of other packets */
> + if (ctx->tx_conf == TLS_SW) {
> + kfree(ctx->tx.rec_seq);
> + kfree(ctx->tx.iv);
> + tls_sw_free_resources_tx(sk);
> + }
>
> - if (ctx->conf == TLS_SW_TX ||
> - ctx->conf == TLS_SW_RX ||
> - ctx->conf == TLS_SW_RXTX) {
> - tls_sw_free_resources(sk);
> + if (ctx->rx_conf == TLS_SW) {
> + kfree(ctx->rx.rec_seq);
> + kfree(ctx->rx.iv);
> + tls_sw_free_resources_rx(sk);
> }
>
> +#ifdef CONFIG_TLS_DEVICE
> + if (ctx->tx_conf != TLS_HW)
> +#endif
> + kfree(ctx);
> +
> +
> skip_tx_cleanup:
> release_sock(sk);
> sk_proto_close(sk, timeout);
> @@ -428,25 +436,29 @@ static int do_tls_setsockopt_conf(struct sock *sk, char __user *optval,
> goto err_crypto_info;
> }
>
> - /* currently SW is default, we will have ethtool in future */
> if (tx) {
> - rc = tls_set_sw_offload(sk, ctx, 1);
> - if (ctx->conf == TLS_SW_RX)
> - conf = TLS_SW_RXTX;
> - else
> - conf = TLS_SW_TX;
> +#ifdef CONFIG_TLS_DEVICE
> + rc = tls_set_device_offload(sk, ctx);
> + conf = TLS_HW;
> + if (rc) {
> +#else
> + {
> +#endif
> + rc = tls_set_sw_offload(sk, ctx, 1);
> + conf = TLS_SW;
> + }
> } else {
> rc = tls_set_sw_offload(sk, ctx, 0);
> - if (ctx->conf == TLS_SW_TX)
> - conf = TLS_SW_RXTX;
> - else
> - conf = TLS_SW_RX;
> + conf = TLS_SW;
> }
>
> if (rc)
> goto err_crypto_info;
>
> - ctx->conf = conf;
> + if (tx)
> + ctx->tx_conf = conf;
> + else
> + ctx->rx_conf = conf;
> update_sk_prot(sk, ctx);
> if (tx) {
> ctx->sk_write_space = sk->sk_write_space;
> @@ -493,24 +505,35 @@ static int tls_setsockopt(struct sock *sk, int level, int optname,
> return do_tls_setsockopt(sk, optname, optval, optlen);
> }
>
> -static void build_protos(struct proto *prot, struct proto *base)
> +static void build_protos(struct proto prot[TLS_NUM_CONFIG][TLS_NUM_CONFIG],
> + struct proto *base)
> {
> - prot[TLS_BASE] = *base;
> - prot[TLS_BASE].setsockopt = tls_setsockopt;
> - prot[TLS_BASE].getsockopt = tls_getsockopt;
> - prot[TLS_BASE].close = tls_sk_proto_close;
> -
> - prot[TLS_SW_TX] = prot[TLS_BASE];
> - prot[TLS_SW_TX].sendmsg = tls_sw_sendmsg;
> - prot[TLS_SW_TX].sendpage = tls_sw_sendpage;
> -
> - prot[TLS_SW_RX] = prot[TLS_BASE];
> - prot[TLS_SW_RX].recvmsg = tls_sw_recvmsg;
> - prot[TLS_SW_RX].close = tls_sk_proto_close;
> -
> - prot[TLS_SW_RXTX] = prot[TLS_SW_TX];
> - prot[TLS_SW_RXTX].recvmsg = tls_sw_recvmsg;
> - prot[TLS_SW_RXTX].close = tls_sk_proto_close;
> + prot[TLS_BASE][TLS_BASE] = *base;
> + prot[TLS_BASE][TLS_BASE].setsockopt = tls_setsockopt;
> + prot[TLS_BASE][TLS_BASE].getsockopt = tls_getsockopt;
> + prot[TLS_BASE][TLS_BASE].close = tls_sk_proto_close;
> +
> + prot[TLS_SW][TLS_BASE] = prot[TLS_BASE][TLS_BASE];
> + prot[TLS_SW][TLS_BASE].sendmsg = tls_sw_sendmsg;
> + prot[TLS_SW][TLS_BASE].sendpage = tls_sw_sendpage;
> +
> + prot[TLS_BASE][TLS_SW] = prot[TLS_BASE][TLS_BASE];
> + prot[TLS_BASE][TLS_SW].recvmsg = tls_sw_recvmsg;
> + prot[TLS_BASE][TLS_SW].close = tls_sk_proto_close;
> +
> + prot[TLS_SW][TLS_SW] = prot[TLS_SW][TLS_BASE];
> + prot[TLS_SW][TLS_SW].recvmsg = tls_sw_recvmsg;
> + prot[TLS_SW][TLS_SW].close = tls_sk_proto_close;
> +
> +#ifdef CONFIG_TLS_DEVICE
> + prot[TLS_HW][TLS_BASE] = prot[TLS_BASE][TLS_BASE];
> + prot[TLS_HW][TLS_BASE].sendmsg = tls_device_sendmsg;
> + prot[TLS_HW][TLS_BASE].sendpage = tls_device_sendpage;
> +
> + prot[TLS_HW][TLS_SW] = prot[TLS_BASE][TLS_SW];
> + prot[TLS_HW][TLS_SW].sendmsg = tls_device_sendmsg;
> + prot[TLS_HW][TLS_SW].sendpage = tls_device_sendpage;
> +#endif
> }
>
> static int tls_init(struct sock *sk)
> @@ -551,7 +574,8 @@ static int tls_init(struct sock *sk)
> mutex_unlock(&tcpv6_prot_mutex);
> }
>
> - ctx->conf = TLS_BASE;
> + ctx->tx_conf = TLS_BASE;
> + ctx->rx_conf = TLS_BASE;
> update_sk_prot(sk, ctx);
> out:
> return rc;
> @@ -573,6 +597,9 @@ static int __init tls_register(void)
> tls_sw_proto_ops.poll = tls_sw_poll;
> tls_sw_proto_ops.splice_read = tls_sw_splice_read;
>
> +#ifdef CONFIG_TLS_DEVICE
> + tls_device_init();
> +#endif
> tcp_register_ulp(&tcp_tls_ulp_ops);
>
> return 0;
> @@ -581,6 +608,9 @@ static int __init tls_register(void)
> static void __exit tls_unregister(void)
> {
> tcp_unregister_ulp(&tcp_tls_ulp_ops);
> +#ifdef CONFIG_TLS_DEVICE
> + tls_device_cleanup();
> +#endif
> }
>
> module_init(tls_register);
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 4dc766b03f00..498b86ab850a 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -50,7 +50,7 @@ static int tls_do_decryption(struct sock *sk,
> gfp_t flags)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> struct strp_msg *rxm = strp_msg(skb);
> struct aead_request *aead_req;
>
> @@ -120,7 +120,7 @@ static void trim_sg(struct sock *sk, struct scatterlist *sg,
> static void trim_both_sgl(struct sock *sk, int target_size)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
>
> trim_sg(sk, ctx->sg_plaintext_data,
> &ctx->sg_plaintext_num_elem,
> @@ -139,7 +139,7 @@ static void trim_both_sgl(struct sock *sk, int target_size)
> static int alloc_encrypted_sg(struct sock *sk, int len)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> int rc = 0;
>
> rc = sk_alloc_sg(sk, len,
> @@ -153,7 +153,7 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
> static int alloc_plaintext_sg(struct sock *sk, int len)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> int rc = 0;
>
> rc = sk_alloc_sg(sk, len, ctx->sg_plaintext_data, 0,
> @@ -179,7 +179,7 @@ static void free_sg(struct sock *sk, struct scatterlist *sg,
> static void tls_free_both_sg(struct sock *sk)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
>
> free_sg(sk, ctx->sg_encrypted_data, &ctx->sg_encrypted_num_elem,
> &ctx->sg_encrypted_size);
> @@ -189,7 +189,7 @@ static void tls_free_both_sg(struct sock *sk)
> }
>
> static int tls_do_encryption(struct tls_context *tls_ctx,
> - struct tls_sw_context *ctx, size_t data_len,
> + struct tls_sw_context_tx *ctx, size_t data_len,
> gfp_t flags)
> {
> unsigned int req_size = sizeof(struct aead_request) +
> @@ -225,7 +225,7 @@ static int tls_push_record(struct sock *sk, int flags,
> unsigned char record_type)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> int rc;
>
> sg_mark_end(ctx->sg_plaintext_data + ctx->sg_plaintext_num_elem - 1);
> @@ -337,7 +337,7 @@ static int memcopy_from_iter(struct sock *sk, struct iov_iter *from,
> int bytes)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> struct scatterlist *sg = ctx->sg_plaintext_data;
> int copy, i, rc = 0;
>
> @@ -365,7 +365,7 @@ static int memcopy_from_iter(struct sock *sk, struct iov_iter *from,
> int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> int ret = 0;
> int required_size;
> long timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT);
> @@ -520,7 +520,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
> int offset, size_t size, int flags)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
> int ret = 0;
> long timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
> bool eor;
> @@ -634,7 +634,7 @@ static struct sk_buff *tls_wait_data(struct sock *sk, int flags,
> long timeo, int *err)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> struct sk_buff *skb;
> DEFINE_WAIT_FUNC(wait, woken_wake_function);
>
> @@ -672,7 +672,7 @@ static int decrypt_skb(struct sock *sk, struct sk_buff *skb,
> struct scatterlist *sgout)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> char iv[TLS_CIPHER_AES_GCM_128_SALT_SIZE + tls_ctx->rx.iv_size];
> struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
> struct scatterlist *sgin = &sgin_arr[0];
> @@ -722,7 +722,7 @@ static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
> unsigned int len)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> struct strp_msg *rxm = strp_msg(skb);
>
> if (len < rxm->full_len) {
> @@ -748,7 +748,7 @@ int tls_sw_recvmsg(struct sock *sk,
> int *addr_len)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> unsigned char control;
> struct strp_msg *rxm;
> struct sk_buff *skb;
> @@ -868,7 +868,7 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos,
> size_t len, unsigned int flags)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sock->sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> struct strp_msg *rxm = NULL;
> struct sock *sk = sock->sk;
> struct sk_buff *skb;
> @@ -921,7 +921,7 @@ unsigned int tls_sw_poll(struct file *file, struct socket *sock,
> unsigned int ret;
> struct sock *sk = sock->sk;
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
>
> /* Grab POLLOUT and POLLHUP from the underlying socket */
> ret = ctx->sk_poll(file, sock, wait);
> @@ -937,7 +937,7 @@ unsigned int tls_sw_poll(struct file *file, struct socket *sock,
> static int tls_read_size(struct strparser *strp, struct sk_buff *skb)
> {
> struct tls_context *tls_ctx = tls_get_ctx(strp->sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> char header[tls_ctx->rx.prepend_size];
> struct strp_msg *rxm = strp_msg(skb);
> size_t cipher_overhead;
> @@ -986,7 +986,7 @@ static int tls_read_size(struct strparser *strp, struct sk_buff *skb)
> static void tls_queue(struct strparser *strp, struct sk_buff *skb)
> {
> struct tls_context *tls_ctx = tls_get_ctx(strp->sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
This is still splitting.
> struct strp_msg *rxm;
>
> rxm = strp_msg(skb);
> @@ -1002,18 +1002,28 @@ static void tls_queue(struct strparser *strp, struct sk_buff *skb)
> static void tls_data_ready(struct sock *sk)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
And this.
> strp_data_ready(&ctx->strp);
> }
>
> -void tls_sw_free_resources(struct sock *sk)
> +void tls_sw_free_resources_tx(struct sock *sk)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> - struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
> + struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
>
> if (ctx->aead_send)
> crypto_free_aead(ctx->aead_send);
> + tls_free_both_sg(sk);
> +
> + kfree(ctx);
> +}
> +
> +void tls_sw_free_resources_rx(struct sock *sk)
> +{
> + struct tls_context *tls_ctx = tls_get_ctx(sk);
> + struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
> +
> if (ctx->aead_recv) {
> if (ctx->recv_pkt) {
> kfree_skb(ctx->recv_pkt);
> @@ -1029,10 +1039,7 @@ void tls_sw_free_resources(struct sock *sk)
> lock_sock(sk);
> }
>
> - tls_free_both_sg(sk);
> -
> kfree(ctx);
> - kfree(tls_ctx);
> }
>
> int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> @@ -1040,7 +1047,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> char keyval[TLS_CIPHER_AES_GCM_128_KEY_SIZE];
> struct tls_crypto_info *crypto_info;
> struct tls12_crypto_info_aes_gcm_128 *gcm_128_info;
> - struct tls_sw_context *sw_ctx;
> + struct tls_sw_context_tx *sw_ctx_tx;
> + struct tls_sw_context_rx *sw_ctx_rx;
> struct cipher_context *cctx;
> struct crypto_aead **aead;
> struct strp_callbacks cb;
> @@ -1053,27 +1061,33 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> goto out;
> }
>
> - if (!ctx->priv_ctx) {
> - sw_ctx = kzalloc(sizeof(*sw_ctx), GFP_KERNEL);
> - if (!sw_ctx) {
> + if (tx) {
> + sw_ctx_tx = kzalloc(sizeof(*sw_ctx_tx), GFP_KERNEL);
> + if (!sw_ctx_tx) {
> rc = -ENOMEM;
> goto out;
> }
> - crypto_init_wait(&sw_ctx->async_wait);
> + crypto_init_wait(&sw_ctx_tx->async_wait);
> + ctx->priv_ctx_tx = sw_ctx_tx;
> } else {
> - sw_ctx = ctx->priv_ctx;
> + sw_ctx_rx = kzalloc(sizeof(*sw_ctx_rx), GFP_KERNEL);
> + if (!sw_ctx_rx) {
> + rc = -ENOMEM;
> + goto out;
> + }
> + crypto_init_wait(&sw_ctx_rx->async_wait);
> + ctx->priv_ctx_rx = sw_ctx_rx;
> }
>
> - ctx->priv_ctx = (struct tls_offload_context *)sw_ctx;
>
> if (tx) {
> crypto_info = &ctx->crypto_send;
> cctx = &ctx->tx;
> - aead = &sw_ctx->aead_send;
> + aead = &sw_ctx_tx->aead_send;
> } else {
> crypto_info = &ctx->crypto_recv;
> cctx = &ctx->rx;
> - aead = &sw_ctx->aead_recv;
> + aead = &sw_ctx_rx->aead_recv;
> }
>
> switch (crypto_info->cipher_type) {
> @@ -1115,21 +1129,22 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> memcpy(cctx->rec_seq, rec_seq, rec_seq_size);
>
> if (tx) {
> - sg_init_table(sw_ctx->sg_encrypted_data,
> - ARRAY_SIZE(sw_ctx->sg_encrypted_data));
> - sg_init_table(sw_ctx->sg_plaintext_data,
> - ARRAY_SIZE(sw_ctx->sg_plaintext_data));
> -
> - sg_init_table(sw_ctx->sg_aead_in, 2);
> - sg_set_buf(&sw_ctx->sg_aead_in[0], sw_ctx->aad_space,
> - sizeof(sw_ctx->aad_space));
> - sg_unmark_end(&sw_ctx->sg_aead_in[1]);
> - sg_chain(sw_ctx->sg_aead_in, 2, sw_ctx->sg_plaintext_data);
> - sg_init_table(sw_ctx->sg_aead_out, 2);
> - sg_set_buf(&sw_ctx->sg_aead_out[0], sw_ctx->aad_space,
> - sizeof(sw_ctx->aad_space));
> - sg_unmark_end(&sw_ctx->sg_aead_out[1]);
> - sg_chain(sw_ctx->sg_aead_out, 2, sw_ctx->sg_encrypted_data);
> + sg_init_table(sw_ctx_tx->sg_encrypted_data,
> + ARRAY_SIZE(sw_ctx_tx->sg_encrypted_data));
> + sg_init_table(sw_ctx_tx->sg_plaintext_data,
> + ARRAY_SIZE(sw_ctx_tx->sg_plaintext_data));
> +
> + sg_init_table(sw_ctx_tx->sg_aead_in, 2);
> + sg_set_buf(&sw_ctx_tx->sg_aead_in[0], sw_ctx_tx->aad_space,
> + sizeof(sw_ctx_tx->aad_space));
> + sg_unmark_end(&sw_ctx_tx->sg_aead_in[1]);
> + sg_chain(sw_ctx_tx->sg_aead_in, 2, sw_ctx_tx->sg_plaintext_data);
> + sg_init_table(sw_ctx_tx->sg_aead_out, 2);
> + sg_set_buf(&sw_ctx_tx->sg_aead_out[0], sw_ctx_tx->aad_space,
> + sizeof(sw_ctx_tx->aad_space));
> + sg_unmark_end(&sw_ctx_tx->sg_aead_out[1]);
> + sg_chain(sw_ctx_tx->sg_aead_out, 2,
> + sw_ctx_tx->sg_encrypted_data);
> }
>
> if (!*aead) {
> @@ -1160,16 +1175,16 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> cb.rcv_msg = tls_queue;
> cb.parse_msg = tls_read_size;
>
> - strp_init(&sw_ctx->strp, sk, &cb);
> + strp_init(&sw_ctx_rx->strp, sk, &cb);
>
> write_lock_bh(&sk->sk_callback_lock);
> - sw_ctx->saved_data_ready = sk->sk_data_ready;
> + sw_ctx_rx->saved_data_ready = sk->sk_data_ready;
> sk->sk_data_ready = tls_data_ready;
> write_unlock_bh(&sk->sk_callback_lock);
>
> - sw_ctx->sk_poll = sk->sk_socket->ops->poll;
> + sw_ctx_rx->sk_poll = sk->sk_socket->ops->poll;
>
> - strp_check_rcv(&sw_ctx->strp);
> + strp_check_rcv(&sw_ctx_rx->strp);
> }
>
> goto out;
> @@ -1181,11 +1196,16 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> kfree(cctx->rec_seq);
> cctx->rec_seq = NULL;
> free_iv:
> - kfree(ctx->tx.iv);
> - ctx->tx.iv = NULL;
> + kfree(cctx->iv);
> + cctx->iv = NULL;
> free_priv:
> - kfree(ctx->priv_ctx);
> - ctx->priv_ctx = NULL;
> + if (tx) {
> + kfree(ctx->priv_ctx_tx);
> + ctx->priv_ctx_tx = NULL;
> + } else {
> + kfree(ctx->priv_ctx_rx);
> + ctx->priv_ctx_rx = NULL;
> + }
> out:
> return rc;
> }
>
./scripts/checkpatch.pl c.diff
WARNING: prefer 'help' over '---help---' for new help texts
#198: FILE: net/tls/Kconfig:18:
+config TLS_DEVICE
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#218:
new file mode 100644
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#223: FILE: net/tls/tls_device.c:1:
+/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
CHECK: Alignment should match open parenthesis
#576: FILE: net/tls/tls_device.c:354:
+ rc = tls_do_allocation(sk, ctx, pfrag,
+ tls_ctx->tx.prepend_size);
CHECK: Alignment should match open parenthesis
#793: FILE: net/tls/tls_device.c:571:
+ ctx->tx.iv = kmalloc(iv_size + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
+ GFP_KERNEL);
WARNING: 'preceed' may be misspelled - perhaps 'precede'?
#841: FILE: net/tls/tls_device.c:619:
+ * This lock must preceed get_netdev_for_sock to prevent races between
WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#988: FILE: net/tls/tls_device_fallback.c:1:
+/* Copyright (c) 2018, Mellanox Technologies All rights reserved.
WARNING: braces {} are not necessary for single statement blocks
#1240: FILE: net/tls/tls_device_fallback.c:253:
+ if (!is_start_marker) {
+ *sync_size = 0;
+ }
CHECK: Blank lines aren't necessary after an open brace '{'
#1277: FILE: net/tls/tls_device_fallback.c:290:
+{
+
CHECK: Please don't use multiple blank lines
#1278: FILE: net/tls/tls_device_fallback.c:291:
+
+
CHECK: Alignment should match open parenthesis
#1330: FILE: net/tls/tls_device_fallback.c:343:
+ if (tls_enc_records(aead_req, ctx->aead_send, sg_in, sg_out, aad, iv,
+ rcd_sn, sync_size + payload_len) < 0)
CHECK: Please don't use multiple blank lines
#1518: FILE: net/tls/tls_main.c:283:
+
+
WARNING: line over 80 characters
#1935: FILE: net/tls/tls_sw.c:1141:
+ sg_chain(sw_ctx_tx->sg_aead_in, 2, sw_ctx_tx->sg_plaintext_data);
total: 0 errors, 7 warnings, 6 checks, 1912 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
c.diff has style problems, please review.
^ permalink raw reply
* Re: [PATCH] [net-next] sctp: fix unused lable warning
From: Neil Horman @ 2018-03-28 15:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Vlad Yasevich, David S. Miller, Al Viro, linux-sctp, netdev,
linux-kernel
In-Reply-To: <20180328141512.3083992-1-arnd@arndb.de>
On Wed, Mar 28, 2018 at 04:14:56PM +0200, Arnd Bergmann wrote:
> The proc file cleanup left a label possibly unused:
>
> net/sctp/protocol.c: In function 'sctp_defaults_init':
> net/sctp/protocol.c:1304:1: error: label 'err_init_proc' defined but not used [-Werror=unused-label]
>
> This adds an #ifdef around it to match the respective 'goto'.
>
> Fixes: d47d08c8ca05 ("sctp: use proc_remove_subtree()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> net/sctp/protocol.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index c1c64b78c39d..d685f8456762 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -1301,8 +1301,10 @@ static int __net_init sctp_defaults_init(struct net *net)
>
> return 0;
>
> +#ifdef CONFIG_PROC_FS
> err_init_proc:
> cleanup_sctp_mibs(net);
> +#endif
> err_init_mibs:
> sctp_sysctl_net_unregister(net);
> err_sysctl_register:
> --
> 2.9.0
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [RFC PATCH v2] net: phy: Added device tree binding for dev-addr and dev-addr code check-up
From: Andrew Lunn @ 2018-03-28 15:27 UTC (permalink / raw)
To: Rob Herring
Cc: Vicenţiu Galanopulo, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, mark.rutland@arm.com,
davem@davemloft.net, marcel@holtmann.org,
devicetree@vger.kernel.org, Madalin-cristian Bucur,
Alexandru Marginean
In-Reply-To: <CAL_JsqJGaGdEgSnnMG8C7P6=gELnuxa-oNgBgK2Yk94HcA=LFg@mail.gmail.com>
> If this is a rare case and something future devices should get right,
> then I'm more inclined to use 'dev-addr' rather than extending reg.
Hi Rob
The sample size is a bit small at the moment to know how rare it is.
I think we have 6 C45 devices so far, and two get this wrong. C45 is
mostly used for PHY device with > 1Gbps. There are not so many of
these yet.
I would also prefer to use 'dev-addr'.
Andrew
^ permalink raw reply
* Re: [PATCH] vhost-net: add time limitation for tx polling(Internet mail)
From: Michael S. Tsirkin @ 2018-03-28 15:31 UTC (permalink / raw)
To: Jason Wang
Cc: haibinzhang(张海斌), kvm@vger.kernel.org,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
lidongchen(陈立东),
yunfangtai(台运方)
In-Reply-To: <0b1846d3-dfcf-df0c-f94c-65d414331d88@redhat.com>
On Wed, Mar 28, 2018 at 02:37:04PM +0800, Jason Wang wrote:
>
>
> On 2018年03月28日 12:01, haibinzhang(张海斌) wrote:
> > On 2018年03月27日 19:26, Jason wrote
> > On 2018年03月27日 17:12, haibinzhang wrote:
> > > > handle_tx() will delay rx for a long time when busy tx polling udp packets
> > > > with short length(ie: 1byte udp payload), because setting VHOST_NET_WEIGHT
> > > > takes into account only sent-bytes but no time.
> > > Interesting.
> > >
> > > Looking at vhost_can_busy_poll() it tries to poke pending vhost work and
> > > exit the busy loop if it found one. So I believe something block the
> > > work queuing. E.g did reverting 8241a1e466cd56e6c10472cac9c1ad4e54bc65db
> > > fix the issue?
> > "busy tx polling" means using netperf send udp packets with 1 bytes payload(total 47bytes frame lenght),
> > and handle_tx() will be busy sending packets continuously.
> >
> > > > It's not fair for handle_rx(),
> > > > so needs to limit max time of tx polling.
> > > >
> > > > ---
> > > > drivers/vhost/net.c | 3 ++-
> > > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > index 8139bc70ad7d..dc9218a3a75b 100644
> > > > --- a/drivers/vhost/net.c
> > > > +++ b/drivers/vhost/net.c
> > > > @@ -473,6 +473,7 @@ static void handle_tx(struct vhost_net *net)
> > > > struct socket *sock;
> > > > struct vhost_net_ubuf_ref *uninitialized_var(ubufs);
> > > > bool zcopy, zcopy_used;
> > > > + unsigned long start = jiffies;
> > > Checking jiffies is tricky, need to convert it to ms or whatever others.
> > >
> > > > mutex_lock(&vq->mutex);
> > > > sock = vq->private_data;
> > > > @@ -580,7 +581,7 @@ static void handle_tx(struct vhost_net *net)
> > > > else
> > > > vhost_zerocopy_signal_used(net, vq);
> > > > vhost_net_tx_packet(net);
> > > > - if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
> > > > + if (unlikely(total_len >= VHOST_NET_WEIGHT) || unlikely(jiffies - start >= 1)) {
> > > How value 1 is determined here? And we need a complete test to make sure
> > > this won't affect other use cases.
> > We just want <1ms ping latency, but actually we are not sure what value is reasonable.
> > We have some test results using netperf before this patch as follow,
> >
> > Udp payload 1byte 100bytes 1000bytes 1400bytes
> > Ping avg latency 25ms 10ms 2ms 1.5ms
> >
> > What is other testcases?
>
> Something like https://patchwork.kernel.org/patch/10151645/.
>
> Btw, you need use time_before() to properly handle jiffies overflow and I
> would also suggest you to try something like #packets limit (e.g 64).
Maybe a ring size?
> For long term, we definitely need more worker threads.
>
> Thanks
Only helps when you have spare CPUs.
> >
> > > Another thought is introduce another limit of #packets, but this need
> > > benchmark too.
> > >
> > > Thanks
> > >
> > > > vhost_poll_queue(&vq->poll);
> > > > break;
> > > > }
> > >
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-28 15:12 UTC (permalink / raw)
To: David Laight, Will Deacon
Cc: Linus Torvalds, Alexander Duyck, Sinan Kaya, Arnd Bergmann,
Jason Gunthorpe, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney,
netdev@vger.kernel.org
In-Reply-To: <a642dc060b484d98bc728eb1f4b5c600@AcuMS.aculab.com>
On Wed, 2018-03-28 at 11:30 +0000, David Laight wrote:
> From: Benjamin Herrenschmidt
> > Sent: 28 March 2018 10:56
>
> ...
> > For example, let's say I have a device with a reset bit and the spec
> > says the reset bit needs to be set for at least 10us.
> >
> > This is wrong:
> >
> > writel(1, RESET_REG);
> > usleep(10);
> > writel(0, RESET_REG);
> >
> > Because of write posting, the first write might arrive to the device
> > right before the second one.
> >
> > The typical "fix" is to turn that into:
> >
> > writel(1, RESET_REG);
> > readl(RESET_REG); /* Flush posted writes */
>
> Would a writel(1, RESET_REG) here provide enough synchronsiation?
Probably yes. It's one of those things where you try to deal with the
fact that 90% of driver writers barely understand the basic stuff and
so you need the "default" accessors to be hardened as much as possible.
We still need to get a reasonably definition of the semantics of the
relaxed ones vs. WC memory but let's get through that exercise first
and hopefully for the last time.
> > usleep(10);
> > writel(0, RESET_REG);
> >
> > *However* the issue here, at least on power, is that the CPU can issue
> > that readl but doesn't necessarily wait for it to complete (ie, the
> > data to return), before proceeding to the usleep. Now a usleep contains
> > a bunch of loads and stores and is probably fine, but a udelay which
> > just loops on the timebase may not be.
> >
> > Thus we may still violate the timing requirement.
>
> I've seem that sort of code (with udelay() and no read back) quite often.
> How many were in linux I don't know.
>
> For small delays I usually fix it by repeated writes (of the same value)
> to the device register. That can guarantee very short intervals.
As long as you know the bus frequency...
> The only time I've actually seen buffered writes break timing was
> between a 286 and an 8859 interrupt controller.
:-)
The problem for me is not so much what I've seen, I realize that most
of the issues we are talking about are the kind that will hit once in a
thousand times or less.
But we *can* reason about them in a way that can effectively prevent
the problem completely and when your cluster has 10000 machine, 1/1000
starts becoming significant.
These days the vast majority of IO devices either are 99% DMA driven so
that a bit of overhead on MMIOs is irrelevant, or have one fast path
(low latency IB etc...) that needs some finer control, and the rest is
all setup which can be paranoid at will.
So I think we should aim for the "default" accessors most people use to
be as hadened as we can think of. I favor correctness over performance
in all cases. But then we also define a reasonable semantic for the
relaxed ones (well, we sort-of do have one, we might have to make it a
bit more precise in some areas) that allows the few MMIO fast path that
care to be optimized.
> If you wrote to the mask then enabled interrupts the first IACK cycle
> could be too close to write and break the cycle recovery time.
> That clobbered most of the interrupt controller registers.
> That probably affected every 286 board ever built!
> Not sure how much software added the required extra bus cycle.
>
> > What we did inside readl, with the twi;isync sequence (which basically
> > means, trap on return value with "trap never" as a condition, followed
> > by isync that ensures all excpetion conditions are resolved), is force
> > the CPU to "consume" the data from the read before moving on.
> >
> > This effectively makes readl fully synchronous (we would probably avoid
> > that if we were to implement a readl_relaxed).
>
> I've always wondered exactly what the twi;isync were for - always seemed
> very heavy handed for most mmio reads.
> Particularly if you are doing mmio reads from a data fifo.
If you do that you should use the "s" version of the accessors. Those
will only do the above trick at the end of the access series. Also a
FIFO needs special care about endianness anyway, so you should use
those accessors regardless. (Hint: you never endian swap a FIFO even on
BE on a LE device, unless something's been wired very badly in HW).
> Perhaps there should be a writel_wait() that is allowed to do a read back
> for such code paths?
I think what we have is fine, we just define that the standard
writel/readl as fairly simple and hardened, and we look at providing a
somewhat reasonable set of relaxed variants for optimizing fast path.
We pretty much already are there, we just need to be better at defining
the semantics.
And for the super high perf case, which thankfully is either seldom
(server high perf network stuff) or very arch specific (ARM SoC stuff),
then arch specific driver hacks will always remain the norm.
Cheers,
Ben.
> David
>
^ permalink raw reply
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Arnd Bergmann @ 2018-03-28 15:43 UTC (permalink / raw)
To: Ioana Ciornei
Cc: gregkh, Laurentiu Tudor, Linux Kernel Mailing List, Stuart Yoder,
Ruxandra Ioana Ciocoi Radulescu, Razvan Stefanescu, Roy Pledge,
Networking
In-Reply-To: <HE1PR04MB32126A07A4B4CC7225E661E4E0A30@HE1PR04MB3212.eurprd04.prod.outlook.com>
On Wed, Mar 28, 2018 at 4:27 PM, Ioana Ciornei <ioana.ciornei@nxp.com> wrote:
> Hi,
>
>>
>> Hi Ioana,
>>
>> So this driver is a direct passthrough to your hardware for passing fixed-
>> length command/response pairs. Have you considered using a higher-level
>> interface instead?
>>
>> Can you list some of the commands that are passed here as clarification, and
>> explain what the tradeoffs are that have led to adopting a low-level interface
>> instead of a high-level interface?
>>
>> The main downside of the direct passthrough obviously is that you tie your
>> user space to a particular hardware implementation, while a high-level
>> abstraction could in principle work across a wider range of hardware revisions
>> or even across multiple vendors implementing the same concept by different
>> means.
>
> If by "higher-level" you mean an implementation where commands are created by the kernel at userspace's request, then I believe this approach is not really viable because of the sheer number of possible commands that would bloat the driver.
>
> For example, a DPNI (Data Path Network Interface) can be created using a command that has the following structure:
>
> struct dpni_cmd_create {
> uint32_t options;
> uint8_t num_queues;
> uint8_t num_tcs;
> uint8_t mac_filter_entries;
> uint8_t pad1;
> uint8_t vlan_filter_entries;
> uint8_t pad2;
> uint8_t qos_entries;
> uint8_t pad3;
> uint16_t fs_entries;
> };
>
> In the above structure, each field has a meaning that the end-user might want to be able to change according to their particular use-case (not much is left at its default value).
> The same level of complexity is encountered for all the commands that interact with Data Path objects such as DPBP(buffer pools), DPRC(Resource Container) etc.
> You can find more examples of commands in restool's repo: https://github.com/qoriq-open-source/restool/tree/integration/mc_v10
>
> In my opinion, an in-kernel implementation that is equivalent in terms of flexibility will turn
> into a giant ioctl parser, all while also exposing an userspace API that is not as simple/easy to use.
(adding the netdev list)
The command you list there seems to be networking related, so instead of
an ioctl based interface, a high-lever interface would likely use netlink
for consistency with other drivers. Are all commands for networking
or are there some that are talking to the device to do something unrelated?
Obviously creating a high-level interface would be a lot of work in the kernel,
and it only pays off if there are multiple independent users, we wouldn't do
that for just one driver.
I'm still not convinced either way (high-level or low-level
interface), but I think
this needs to be discussed with the networking maintainers. Given the examples
on the github page you linked to, the high-level user space commands
based on these ioctls
ls-addni # adds a network interface
ls-addmux # adds a dpdmux
ls-addsw # adds an l2switch
ls-listmac # lists MACs and their connections
ls-listni # lists network interfaces and their connections
and I see that you also support the switchdev interface in
drivers/staging/fsl-dpaa2, which I think does some of the same
things, presumably by implementing the switchdev API using
fsl_mc_command low-level interfaces in the kernel.
Is that a correct interpretation? If yes, could we extend switchdev
or other networking interfaces to fill in whatever those don't handle
yet?
>> > @@ -14,10 +14,18 @@
>> > * struct fsl_mc_command - Management Complex (MC) command
>> structure
>> > * @header: MC command header
>> > * @params: MC command parameters
>> > + *
>> > + * Used by RESTOOL_SEND_MC_COMMAND
>> > */
>> > struct fsl_mc_command {
>> > __u64 header;
>> > __u64 params[MC_CMD_NUM_OF_PARAMS]; };
>> >
>> > +#define RESTOOL_IOCTL_TYPE 'R'
>> > +#define RESTOOL_IOCTL_SEQ 0xE0
>>
>> I tried to follow the code path into the hardware and am a bit confused about
>> the semantics of the 'header' field and the data. Both are accessed passed to
>> the hardware using
>>
>> writeq(le64_to_cpu(cmd->header))
>>
>> which would indicate a fixed byte layout on the user structure and that it
>> should really be a '__le64' instead of '__u64', or possibly should be
>> represented as '__u8 header[8]' to clarify that the byte ordering is supposed
>> to match the order of the byte addresses of the register.
>>
>
> Indeed, the struct fsl_mc_command should use '__le64' instead of '__u64', so that the UAPI header file clearly states what endianness should the userspace prepare.
> The writeq appeared as a consequence of a previous mail thread https://lkml.org/lkml/2017/7/17/415, because the endianness of the command is handled by the user and the bus should leave the binary blob intact.
Ok. However, with the dpni_cmd_create structure you list above, it
seems that it doesn't actually contain a set of __le64 but rather
an array of bytes, so it might be best to
>> However, the in-kernel usage of that field suggests that we treat it as a 64-bit
>> cpu-endian number, for which the hardware needs to know the endianess of
>> the currently running kernel and user space.
>>
>
> I might have not understood what you are trying to say, but the in-kernel code treats the fsl_mc_command as little endian by using cpu_to_leXX and leXX_to_cpu in order to setup the command structure.
One function that seems to have a problem is this one:
static enum mc_cmd_status mc_cmd_hdr_read_status(struct fsl_mc_command *cmd)
{
struct mc_cmd_header *hdr = (struct mc_cmd_header *)&cmd->header;
return (enum mc_cmd_status)hdr->status;
}
If cmd->header is little-endian here, then the result is not
an 'enum mc_cmd_status'. I think it would be good to change the
types for fsl_mc_command to __le64 first and run everything through
'sparse' with 'make C=1' to find any remaining endianess problems.
Arnd
^ 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