* 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
* Re: [bpf-next PATCH v2 3/4] bpf: sockmap, BPF_F_INGRESS flag for BPF_SK_SKB_STREAM_VERDICT:
From: John Fastabend @ 2018-03-28 15:45 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: netdev, davem
In-Reply-To: <71a13f21-6886-85c3-6911-8ac33c486901@iogearbox.net>
On 03/28/2018 07:21 AM, Daniel Borkmann wrote:
> 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?
Correct.
>
>> + 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?
>
Actually, I'll just move the sk_mem_charge() down below this error
then we don't need to unwind it.
>> + 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: NFS mounts failing when keytab present on client
From: J. Bruce Fields @ 2018-03-28 15:46 UTC (permalink / raw)
To: Eric Biggers
Cc: Michael Young, Herbert Xu, Jeff Layton, Trond Myklebust,
Anna Schumaker, linux-nfs, netdev, linux-kernel
In-Reply-To: <20180327222950.GB257332@google.com>
On Tue, Mar 27, 2018 at 03:29:50PM -0700, Eric Biggers wrote:
> Hi Michael,
>
> On Tue, Mar 27, 2018 at 11:06:14PM +0100, Michael Young wrote:
> > NFS mounts stopped working on one of my computers after a kernel update from
> > 4.15.3 to 4.15.4. I traced the problem to the commit
> > [46e8d06e423c4f35eac7a8b677b713b3ec9b0684] crypto: hash - prevent using
> > keyed hashes without setting key
> > and a later kernel with this patch reverted works normally.
> >
> > The problem seems to be related to kerberos as the mount fails when the
> > keytab is present, but works if I rename the keytab file. This is true even
> > though the mount is with sec=sys . The mount should also work with sec=krb5
> > but that also fails in the same way. When the mount fails there are errors
> > in dmesg like
> > [ 1232.522816] gss_marshal: gss_get_mic FAILED (851968)
> > [ 1232.522819] RPC: couldn't encode RPC header, exit EIO
> > [ 1232.522856] gss_marshal: gss_get_mic FAILED (851968)
> > [ 1232.522857] RPC: couldn't encode RPC header, exit EIO
> > [ 1232.522863] NFS: nfs4_discover_server_trunking unhandled error -5.
> > Exiting with error EIO
> > [ 1232.525039] gss_marshal: gss_get_mic FAILED (851968)
> > [ 1232.525042] RPC: couldn't encode RPC header, exit EIO
> >
> > Michael Young
>
> Thanks for the bug report. I think the error is coming from
> net/sunrpc/auth_gss/gss_krb5_crypto.c. There are two potential problems I see.
> The first one, which is definitely a bug, is that make_checksum_hmac_md5()
> allocates an HMAC transform and request, then does these crypto API calls:
>
> crypto_ahash_init()
> crypto_ahash_setkey()
> crypto_ahash_digest()
>
> This is wrong because it makes no sense to init() the HMAC request before the
> key has been set, and doubly so when it's calling digest() which is shorthand
> for init() + update() + final(). So I think it just needs to be removed. You
> can test the following patch:
When was this introduced?
3b5cf20cf439 "sunrpc: Use skcipher and ahash/shash"
- probably not, assuming the above was still just as wrong with
crypto_hash_{init,setkey,digest} as it is with
crypto_ahash_{init,setkey,digest}
So I'm guessing it was wrong from the start when it was added by
fffdaef2eb4a "gss_krb5: Add support for rc4-hmac encryption" 8 years
ago. Wonder why it took this long to notice? Did something else
change?
--b.
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_crypto.c b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> index 12649c9fedab..8654494b4d0a 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_crypto.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_crypto.c
> @@ -237,9 +237,6 @@ make_checksum_hmac_md5(struct krb5_ctx *kctx, char *header, int hdrlen,
>
> ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP, NULL, NULL);
>
> - err = crypto_ahash_init(req);
> - if (err)
> - goto out;
> err = crypto_ahash_setkey(hmac_md5, cksumkey, kctx->gk5e->keylength);
> if (err)
> goto out;
>
> If that's not it, it's also possible that the error is coming from the
> crypto_ahash_init() in make_checksum(). That can only happen if 'cksumkey' is
> NULL and the hash algorithm is keyed, which implies a logical error as it
> doesn't make sense to use a keyed hash algorithm without the key. The callers
> do check kctx->gk5e->keyed_cksum which I'd hope would prevent this, though
> perhaps kctx->cksum can be NULL.
>
> Eric
^ 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 15:46 UTC (permalink / raw)
To: John Fastabend, ast; +Cc: netdev, davem
In-Reply-To: <3e7dd92b-a932-d977-2a7e-505b374733df@gmail.com>
On 03/28/2018 05:45 PM, John Fastabend wrote:
> On 03/28/2018 07:21 AM, Daniel Borkmann wrote:
>> 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?
>
> Correct.
>
>>
>>> + 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?
>>
>
> Actually, I'll just move the sk_mem_charge() down below this error
> then we don't need to unwind it.
Agree, good point.
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-28 15:13 UTC (permalink / raw)
To: okaya, Linus Torvalds
Cc: Alexander Duyck, Will Deacon, Arnd Bergmann, Jason Gunthorpe,
David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Paul E. McKenney, netdev, linus971
In-Reply-To: <b02cc752d90c5e5ae2cc8cc4f67429a7@codeaurora.org>
On Wed, 2018-03-28 at 07:41 -0400, okaya@codeaurora.org wrote:
> Yes, we want to get there indeed. It is because of some arch not
> implementing writel properly. Maintainers want to play safe.
>
> That is why I asked if IA64 and other well known archs follow the
> strongly ordered rule at this moment like PPC and ARM.
>
> Or should we go and inform every arch about this before yanking wmb()?
>
> Maintainers are afraid of introducing a regression.
Let's fix all archs, it's way easier than fixing all drivers. Half of
the archs are unused or dead anyway.
> >
> > The above code makes no sense, and just looks stupid to me. It also
> > generates pointlessly bad code on x86, so it's bad there too.
> >
> > Linus
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: David Miller @ 2018-03-28 15:55 UTC (permalink / raw)
To: benh
Cc: okaya, torvalds, alexander.duyck, will.deacon, arnd, jgg,
David.Laight, oohall, linuxppc-dev, linux-rdma, alexander.h.duyck,
paulmck, netdev, linus971
In-Reply-To: <1522249996.21446.25.camel@kernel.crashing.org>
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Thu, 29 Mar 2018 02:13:16 +1100
> Let's fix all archs, it's way easier than fixing all drivers. Half of
> the archs are unused or dead anyway.
Agreed.
^ permalink raw reply
* [PATCH] gfs2: Stop using rhashtable_walk_peek
From: Andreas Gruenbacher @ 2018-03-28 16:00 UTC (permalink / raw)
To: cluster-devel
Cc: Andreas Gruenbacher, netdev, linux-kernel, NeilBrown, Thomas Graf,
Herbert Xu, Tom Herbert
Function rhashtable_walk_peek is problematic because there is no
guarantee that the glock previously returned still exists; when that key
is deleted, rhashtable_walk_peek can end up returning a different key,
which would cause an inconsistent glock dump. So instead of using
rhashtable_walk_peek, keep track of the current glock in the seq file
iterator functions.
Signed-off-by: Andreas Gruenbacher <agruenba@redhat.com>
---
fs/gfs2/glock.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/fs/gfs2/glock.c b/fs/gfs2/glock.c
index 82fb5583445c..f1fc353875d3 100644
--- a/fs/gfs2/glock.c
+++ b/fs/gfs2/glock.c
@@ -55,6 +55,7 @@ struct gfs2_glock_iter {
struct gfs2_sbd *sdp; /* incore superblock */
struct rhashtable_iter hti; /* rhashtable iterator */
struct gfs2_glock *gl; /* current glock struct */
+ bool gl_held;
loff_t last_pos; /* last position */
};
@@ -1923,9 +1924,11 @@ void gfs2_glock_exit(void)
static void gfs2_glock_iter_next(struct gfs2_glock_iter *gi, loff_t n)
{
- if (n == 0)
- gi->gl = rhashtable_walk_peek(&gi->hti);
- else {
+ if (n != 0 || !gi->gl) {
+ if (gi->gl_held) {
+ gfs2_glock_queue_put(gi->gl);
+ gi->gl_held = false;
+ }
gi->gl = rhashtable_walk_next(&gi->hti);
n--;
}
@@ -1988,7 +1991,10 @@ static void gfs2_glock_seq_stop(struct seq_file *seq, void *iter_ptr)
{
struct gfs2_glock_iter *gi = seq->private;
- gi->gl = NULL;
+ if (gi->gl) {
+ lockref_get(&gi->gl->gl_lockref);
+ gi->gl_held = true;
+ }
rhashtable_walk_stop(&gi->hti);
}
@@ -2061,6 +2067,7 @@ static int __gfs2_glocks_open(struct inode *inode, struct file *file,
*/
gi->last_pos = -1;
gi->gl = NULL;
+ gi->gl_held = false;
rhashtable_walk_enter(&gl_hash_table, &gi->hti);
}
return ret;
@@ -2076,7 +2083,8 @@ static int gfs2_glocks_release(struct inode *inode, struct file *file)
struct seq_file *seq = file->private_data;
struct gfs2_glock_iter *gi = seq->private;
- gi->gl = NULL;
+ if (gi->gl_held)
+ gfs2_glock_put(gi->gl);
rhashtable_walk_exit(&gi->hti);
return seq_release_private(inode, file);
}
--
2.14.3
^ permalink raw reply related
* RE: RFC on writel and writel_relaxed
From: David Laight @ 2018-03-28 16:16 UTC (permalink / raw)
To: 'Benjamin Herrenschmidt', 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: <1522249950.21446.23.camel@kernel.crashing.org>
From: Benjamin Herrenschmidt
> Sent: 28 March 2018 16:13
...
> > 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).
That was actually a 64 bit wide fifo connected to a 16bit wide PIO interface.
Reading the high address 'clocked' the fifo.
So the first 3 reads could happen in any order, but the 4th had to be last.
This is a small ppc and we shovel a lot of data through that fifo.
Whether it needed byteswapping depended completely on how our hardware people
had built the pcb (not made easy by some docs using the ibm bit numbering).
In fact it didn't....
While that driver only had to run on a very specific small ppc, generic drivers
might have similar issues.
I suspect that writel() is always (or should always be):
barrier_before_writel()
writel_relaxed()
barrier_after_writel()
So if a driver needs to do multiple writes (without strong ordering)
it should be able to repeat the writel_relaxed() with only one set
of barriers.
Similarly for readl().
In addition a lesser barrier is probably enough between a readl_relaxed()
and a writel_relaxed() that is conditional on the read value.
David
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: phy: Add initial support for Microsemi Ocelot internal PHYs.
From: Alexandre Belloni @ 2018-03-28 16:18 UTC (permalink / raw)
To: Florian Fainelli
Cc: David S . Miller, Allan Nielsen, razvan.stefanescu, po.liu,
Thomas Petazzoni, Andrew Lunn, netdev, devicetree, linux-kernel,
linux-mips, Raju Lakkaraju
In-Reply-To: <1caa7359-9ca3-be22-fae0-475daf8b220f@gmail.com>
On 23/03/2018 at 14:08:10 -0700, Florian Fainelli wrote:
> On 03/23/2018 01:11 PM, Alexandre Belloni wrote:
> > Add Microsemi Ocelot internal PHY ids. For now, simply use the genphy
> > functions but more features are available.
> >
> > Cc: Raju Lakkaraju <Raju.Lakkaraju@microsemi.com>
> > Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> > ---
> > drivers/net/phy/mscc.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
> > index 650c2667d523..e1ab3acd1cdb 100644
> > --- a/drivers/net/phy/mscc.c
> > +++ b/drivers/net/phy/mscc.c
> > @@ -91,6 +91,7 @@ enum rgmii_rx_clock_delay {
> > #define SECURE_ON_PASSWD_LEN_4 0x4000
> >
> > /* Microsemi PHY ID's */
> > +#define PHY_ID_OCELOT 0x00070540
> > #define PHY_ID_VSC8530 0x00070560
> > #define PHY_ID_VSC8531 0x00070570
> > #define PHY_ID_VSC8540 0x00070760
> > @@ -658,6 +659,19 @@ static int vsc85xx_probe(struct phy_device *phydev)
> >
> > /* Microsemi VSC85xx PHYs */
> > static struct phy_driver vsc85xx_driver[] = {
> > +{
> > + .phy_id = PHY_ID_OCELOT,
> > + .name = "Microsemi OCELOT",
> > + .phy_id_mask = 0xfffffff0,
> > + .features = PHY_GBIT_FEATURES,
> > + .soft_reset = &genphy_soft_reset,
> > + .config_init = &genphy_config_init,
> > + .config_aneg = &genphy_config_aneg,
> > + .aneg_done = &genphy_aneg_done,
> > + .read_status = &genphy_read_status,
> > + .suspend = &genphy_suspend,
> > + .resume = &genphy_resume,
>
> With the exception of config_init(), suspend and resume, everything else
> is already the default when you don't provide a callback. To echo to
> what Andrew wrote already, if the purpose is just to show a nice name,
> and do nothing else, consider using the Generic PHY driver (default).
Ok, I'll drop this patch for now, until we handle more features for that
PHY.
--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] sfp: allow cotsworks modules
From: Joe Perches @ 2018-03-28 16:19 UTC (permalink / raw)
To: Russell King - ARM Linux; +Cc: Andrew Lunn, Florian Fainelli, netdev
In-Reply-To: <20180328104147.GO10980@n2100.armlinux.org.uk>
On Wed, 2018-03-28 at 11:41 +0100, Russell King - ARM Linux wrote:
> On Wed, Mar 28, 2018 at 03:33:57AM -0700, Joe Perches wrote:
> > On Wed, 2018-03-28 at 11:18 +0100, Russell King wrote:
> > > Cotsworks modules fail the checksums - it appears that Cotsworks
> > > reprograms the EEPROM at the end of production with the final product
> > > information (serial, date code, and exact part number for module
> > > options) and fails to update the checksum.
> >
> > trivia:
> >
> > > diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
> >
> > []
> > > @@ -574,23 +575,43 @@ static int sfp_sm_mod_probe(struct sfp *sfp)
> >
> > []
> > > + if (cotsworks) {
> > > + dev_warn(sfp->dev,
> > > + "EEPROM base structure checksum failure (0x%02x != 0x%02x)\n",
> > > + check, id.base.cc_base);
> > > + } else {
> > > + dev_err(sfp->dev,
> > > + "EEPROM base structure checksum failure: 0x%02x != 0x%02x\n",
> >
> > It'd be better to move this above the if and
> > use only a single format string instead of
> > using 2 slightly different formats.
>
> No. I think you've missed the fact that one is a _warning_ the other is
> an _error_ and they are emitted at the appropriate severity. It's not
> just that the format strings are slightly different.
Right. Still nicer to use the same formats.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
From: Yonghong Song @ 2018-03-28 16:19 UTC (permalink / raw)
To: Dan Carpenter, Alexei Starovoitov
Cc: Daniel Borkmann, netdev, kernel-janitors
In-Reply-To: <20180328114836.GD29050@mwanda>
On 3/28/18 4:48 AM, Dan Carpenter wrote:
> The skb_segment() function returns error pointers on error. It never
> returns NULL.
>
> Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/lib/test_bpf.c b/lib/test_bpf.c
> index b2badf6b23cd..8e157806df7a 100644
> --- a/lib/test_bpf.c
> +++ b/lib/test_bpf.c
> @@ -6649,7 +6649,7 @@ static __init int test_skb_segment(void)
> }
>
> segs = skb_segment(skb, features);
> - if (segs) {
> + if (!IS_ERR(segs)) {
> kfree_skb_list(segs);
> ret = 0;
> pr_info("%s: success in skb_segment!", __func__);
Oh, my bad. Thanks for the fix!
Reviewed-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Nicholas Piggin @ 2018-03-28 16:23 UTC (permalink / raw)
To: David Miller
Cc: benh, paulmck, arnd, linux-rdma, linuxppc-dev, linus971,
will.deacon, alexander.duyck, okaya, jgg, David.Laight, oohall,
netdev, alexander.h.duyck, torvalds
In-Reply-To: <20180328.115509.481837809903086401.davem@davemloft.net>
On Wed, 28 Mar 2018 11:55:09 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Thu, 29 Mar 2018 02:13:16 +1100
>
> > Let's fix all archs, it's way easier than fixing all drivers. Half of
> > the archs are unused or dead anyway.
>
> Agreed.
While we're making decrees here, can we do something about mmiowb?
The semantics are basically indecipherable.
This is a variation on the mandatory write barrier that causes writes to weakly
ordered I/O regions to be partially ordered. Its effects may go beyond the
CPU->Hardware interface and actually affect the hardware at some level.
How can a driver writer possibly get that right?
IIRC it was added for some big ia64 system that was really expensive
to implement the proper wmb() semantics on. So wmb() semantics were
quietly downgraded, then the subsequently broken drivers they cared
about were fixed by adding the stronger mmiowb().
What should have happened was wmb and writel remained correct, sane, and
expensive, and they add an mmio_wmb() to order MMIO stores made by the
writel_relaxed accessors, then use that to speed up the few drivers they
care about.
Now that ia64 doesn't matter too much, can we deprecate mmiowb and just
make wmb ordering talk about stores to the device, not to some
intermediate stage of the interconnect where it can be subsequently
reordered wrt the device? Drivers can be converted back to using wmb
or writel gradually.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH net-next] net: bcmgenet: return NULL instead of plain integer
From: Florian Fainelli @ 2018-03-28 16:24 UTC (permalink / raw)
To: Wei Yongjun, Doug Berger; +Cc: netdev, kernel-janitors
In-Reply-To: <1522241479-77691-1-git-send-email-weiyongjun1@huawei.com>
On March 28, 2018 5:51:19 AM PDT, Wei Yongjun <weiyongjun1@huawei.com> wrote:
>Fixes the following sparse warning:
>
>drivers/net/ethernet/broadcom/genet/bcmgenet.c:1351:16: warning:
> Using plain integer as NULL pointer
>
>Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH bpf-next RFC 1/2] Organize MPLS headers
From: Shrijeet Mukherjee @ 2018-03-28 16:27 UTC (permalink / raw)
To: daniel; +Cc: netdev, roopa, dsa, ast
From: Shrijeet Mukherjee <shrijeet@gmail.com>
Prepare shared headers for new MPLS label push/pop EBPF helpers
Signed-off-by: Shrijeet Mukherjee <shm@cumulusnetworks.com>
---
include/net/mpls.h | 1 +
net/core/filter.c | 2 ++
net/mpls/af_mpls.c | 8 ++++----
net/mpls/internal.h | 31 -------------------------------
net/mpls/mpls_iptunnel.c | 2 +-
net/openvswitch/actions.c | 12 ++++++------
6 files changed, 14 insertions(+), 42 deletions(-)
diff --git a/include/net/mpls.h b/include/net/mpls.h
index 1dbc669b770e..2583dbc689b8 100644
--- a/include/net/mpls.h
+++ b/include/net/mpls.h
@@ -16,6 +16,7 @@
#include <linux/if_ether.h>
#include <linux/netdevice.h>
+#include <uapi/linux/mpls.h>
#define MPLS_HLEN 4
diff --git a/net/core/filter.c b/net/core/filter.c
index c86f03fd9ea5..00f62fafc788 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -29,6 +29,7 @@
#include <linux/sock_diag.h>
#include <linux/in.h>
#include <linux/inet.h>
+#include <linux/mpls.h>
#include <linux/netdevice.h>
#include <linux/if_packet.h>
#include <linux/if_arp.h>
@@ -56,6 +57,7 @@
#include <net/sock_reuseport.h>
#include <net/busy_poll.h>
#include <net/tcp.h>
+#include <net/mpls.h>
#include <linux/bpf_trace.h>
/**
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c
index d4a89a8be013..4e05391b77f0 100644
--- a/net/mpls/af_mpls.c
+++ b/net/mpls/af_mpls.c
@@ -170,7 +170,7 @@ static u32 mpls_multipath_hash(struct mpls_route *rt, struct sk_buff *skb)
break;
/* Read and decode the current label */
- hdr = mpls_hdr(skb) + label_index;
+ hdr = skb_mpls_hdr(skb) + label_index;
dec = mpls_entry_decode(hdr);
/* RFC6790 - reserved labels MUST NOT be used as keys
@@ -379,7 +379,7 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
goto err;
/* Read and decode the label */
- hdr = mpls_hdr(skb);
+ hdr = skb_mpls_hdr(skb);
dec = mpls_entry_decode(hdr);
rt = mpls_route_input_rcu(net, dec.label);
@@ -440,7 +440,7 @@ static int mpls_forward(struct sk_buff *skb, struct net_device *dev,
skb_push(skb, new_header_size);
skb_reset_network_header(skb);
/* Push the new labels */
- hdr = mpls_hdr(skb);
+ hdr = skb_mpls_hdr(skb);
bos = dec.bos;
for (i = nh->nh_labels - 1; i >= 0; i--) {
hdr[i] = mpls_entry_encode(nh->nh_label[i],
@@ -2203,7 +2203,7 @@ static int mpls_getroute(struct sk_buff *in_skb, struct nlmsghdr *in_nlh,
skb_reset_network_header(skb);
/* Push new labels */
- hdr = mpls_hdr(skb);
+ hdr = skb_mpls_hdr(skb);
bos = true;
for (i = n_labels - 1; i >= 0; i--) {
hdr[i] = mpls_entry_encode(labels[i],
diff --git a/net/mpls/internal.h b/net/mpls/internal.h
index 768a302879b4..cd93cb201fed 100644
--- a/net/mpls/internal.h
+++ b/net/mpls/internal.h
@@ -8,13 +8,6 @@
*/
#define MAX_NEW_LABELS 30
-struct mpls_entry_decoded {
- u32 label;
- u8 ttl;
- u8 tc;
- u8 bos;
-};
-
struct mpls_pcpu_stats {
struct mpls_link_stats stats;
struct u64_stats_sync syncp;
@@ -172,30 +165,6 @@ struct mpls_route { /* next hop label forwarding entry */
#define endfor_nexthops(rt) }
-static inline struct mpls_shim_hdr mpls_entry_encode(u32 label, unsigned ttl, unsigned tc, bool bos)
-{
- struct mpls_shim_hdr result;
- result.label_stack_entry =
- cpu_to_be32((label << MPLS_LS_LABEL_SHIFT) |
- (tc << MPLS_LS_TC_SHIFT) |
- (bos ? (1 << MPLS_LS_S_SHIFT) : 0) |
- (ttl << MPLS_LS_TTL_SHIFT));
- return result;
-}
-
-static inline struct mpls_entry_decoded mpls_entry_decode(struct mpls_shim_hdr *hdr)
-{
- struct mpls_entry_decoded result;
- unsigned entry = be32_to_cpu(hdr->label_stack_entry);
-
- result.label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
- result.ttl = (entry & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
- result.tc = (entry & MPLS_LS_TC_MASK) >> MPLS_LS_TC_SHIFT;
- result.bos = (entry & MPLS_LS_S_MASK) >> MPLS_LS_S_SHIFT;
-
- return result;
-}
-
static inline struct mpls_dev *mpls_dev_get(const struct net_device *dev)
{
return rcu_dereference_rtnl(dev->mpls_ptr);
diff --git a/net/mpls/mpls_iptunnel.c b/net/mpls/mpls_iptunnel.c
index 6e558a419f60..f8346dfba9c8 100644
--- a/net/mpls/mpls_iptunnel.c
+++ b/net/mpls/mpls_iptunnel.c
@@ -127,7 +127,7 @@ static int mpls_xmit(struct sk_buff *skb)
skb->protocol = htons(ETH_P_MPLS_UC);
/* Push the new labels */
- hdr = mpls_hdr(skb);
+ hdr = skb_mpls_hdr(skb);
bos = true;
for (i = tun_encap_info->labels - 1; i >= 0; i--) {
hdr[i] = mpls_entry_encode(tun_encap_info->label[i],
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 30a5df27116e..c6fdc632e6fb 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -205,7 +205,7 @@ static int push_mpls(struct sk_buff *skb, struct sw_flow_key *key,
skb_reset_mac_header(skb);
skb_set_network_header(skb, skb->mac_len);
- new_mpls_lse = mpls_hdr(skb);
+ new_mpls_lse = skb_mpls_hdr(skb);
new_mpls_lse->label_stack_entry = mpls->mpls_lse;
skb_postpush_rcsum(skb, new_mpls_lse, MPLS_HLEN);
@@ -227,7 +227,7 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
if (unlikely(err))
return err;
- skb_postpull_rcsum(skb, mpls_hdr(skb), MPLS_HLEN);
+ skb_postpull_rcsum(skb, skb_mpls_hdr(skb), MPLS_HLEN);
memmove(skb_mac_header(skb) + MPLS_HLEN, skb_mac_header(skb),
skb->mac_len);
@@ -239,10 +239,10 @@ static int pop_mpls(struct sk_buff *skb, struct sw_flow_key *key,
if (ovs_key_mac_proto(key) == MAC_PROTO_ETHERNET) {
struct ethhdr *hdr;
- /* mpls_hdr() is used to locate the ethertype field correctly in the
- * presence of VLAN tags.
+ /* skb_mpls_hdr() is used to locate the ethertype field
+ * correctly in the presence of VLAN tags.
*/
- hdr = (struct ethhdr *)((void *)mpls_hdr(skb) - ETH_HLEN);
+ hdr = (struct ethhdr *)((void *)skb_mpls_hdr(skb) - ETH_HLEN);
update_ethertype(skb, hdr, ethertype);
}
if (eth_p_mpls(skb->protocol))
@@ -263,7 +263,7 @@ static int set_mpls(struct sk_buff *skb, struct sw_flow_key *flow_key,
if (unlikely(err))
return err;
- stack = mpls_hdr(skb);
+ stack = skb_mpls_hdr(skb);
lse = OVS_MASKED(stack->label_stack_entry, *mpls_lse, *mask);
if (skb->ip_summed == CHECKSUM_COMPLETE) {
__be32 diff[] = { ~(stack->label_stack_entry), lse };
--
2.16.2.windows.1
^ permalink raw reply related
* [PATCH bpf-next RFC 2/2] Add MPLS label push/pop functions for EBPF
From: Shrijeet Mukherjee @ 2018-03-28 16:27 UTC (permalink / raw)
To: daniel; +Cc: netdev, roopa, dsa, ast
In-Reply-To: <20180328162715.113-1-shrijeetoss@gmail.com>
From: Shrijeet Mukherjee <shrijeet@gmail.com>
Push: takes an array/size of labels and encaps
Pop : Takes a number and pops the top "n" labels
Signed-off-by: Shrijeet Mukherjee <shm@cumulusnetworks.com>
PS: Will be adding tests as well, refactoring my test into something
smaller.
---
include/linux/bpf.h | 2 +
include/net/mpls.h | 38 +++++++++++++-
include/uapi/linux/bpf.h | 11 +++-
net/core/filter.c | 127 ++++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 175 insertions(+), 3 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 819229c80eca..b38bfabc1fb5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -672,6 +672,8 @@ extern const struct bpf_func_proto bpf_get_current_uid_gid_proto;
extern const struct bpf_func_proto bpf_get_current_comm_proto;
extern const struct bpf_func_proto bpf_skb_vlan_push_proto;
extern const struct bpf_func_proto bpf_skb_vlan_pop_proto;
+extern const struct bpf_func_proto bpf_skb_mpls_push_proto;
+extern const struct bpf_func_proto bpf_skb_mpls_pop_proto;
extern const struct bpf_func_proto bpf_get_stackid_proto;
extern const struct bpf_func_proto bpf_sock_map_update_proto;
diff --git a/include/net/mpls.h b/include/net/mpls.h
index 2583dbc689b8..3a5b8c00823d 100644
--- a/include/net/mpls.h
+++ b/include/net/mpls.h
@@ -24,14 +24,50 @@ struct mpls_shim_hdr {
__be32 label_stack_entry;
};
+struct mpls_entry_decoded {
+ u32 label;
+ u8 ttl;
+ u8 tc;
+ u8 bos;
+};
+
static inline bool eth_p_mpls(__be16 eth_type)
{
return eth_type == htons(ETH_P_MPLS_UC) ||
eth_type == htons(ETH_P_MPLS_MC);
}
-static inline struct mpls_shim_hdr *mpls_hdr(const struct sk_buff *skb)
+static inline struct mpls_shim_hdr *skb_mpls_hdr(const struct sk_buff *skb)
{
return (struct mpls_shim_hdr *)skb_network_header(skb);
}
+
+static inline struct mpls_shim_hdr
+mpls_entry_encode(u32 label, unsigned int ttl, unsigned int tc, bool bos)
+{
+ struct mpls_shim_hdr result;
+
+ result.label_stack_entry =
+ cpu_to_be32((label << MPLS_LS_LABEL_SHIFT)
+ | (tc << MPLS_LS_TC_SHIFT)
+ | (bos ? (1 << MPLS_LS_S_SHIFT) : 0)
+ | (ttl << MPLS_LS_TTL_SHIFT));
+
+ return result;
+}
+
+static inline
+struct mpls_entry_decoded mpls_entry_decode(struct mpls_shim_hdr *hdr)
+{
+ struct mpls_entry_decoded result;
+ unsigned int entry = be32_to_cpu(hdr->label_stack_entry);
+
+ result.label = (entry & MPLS_LS_LABEL_MASK) >> MPLS_LS_LABEL_SHIFT;
+ result.ttl = (entry & MPLS_LS_TTL_MASK) >> MPLS_LS_TTL_SHIFT;
+ result.tc = (entry & MPLS_LS_TC_MASK) >> MPLS_LS_TC_SHIFT;
+ result.bos = (entry & MPLS_LS_S_MASK) >> MPLS_LS_S_SHIFT;
+
+ return result;
+}
+
#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 18b7c510c511..2278548e1f8f 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -439,6 +439,12 @@ union bpf_attr {
* int bpf_skb_vlan_pop(skb)
* Return: 0 on success or negative error
*
+ * int bpf_skb_mpls_push(skb, num_lbls, lbls[])
+ * Return 0 on success or negative error
+ *
+ * int bpf_skb_mpls_pop(skb, num_lbls)
+ * Return number of popped labels, 0 is no-op, deliver packet to current dst
+ *
* int bpf_skb_get_tunnel_key(skb, key, size, flags)
* int bpf_skb_set_tunnel_key(skb, key, size, flags)
* retrieve or populate tunnel metadata
@@ -794,7 +800,10 @@ union bpf_attr {
FN(msg_redirect_map), \
FN(msg_apply_bytes), \
FN(msg_cork_bytes), \
- FN(msg_pull_data),
+ FN(msg_pull_data), \
+ FN(skb_mpls_push), \
+ FN(skb_mpls_pop),
+
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/net/core/filter.c b/net/core/filter.c
index 00f62fafc788..c96ae8ef423d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2522,7 +2522,7 @@ static int bpf_skb_adjust_net(struct sk_buff *skb, s32 len_diff)
}
BPF_CALL_4(bpf_skb_adjust_room, struct sk_buff *, skb, s32, len_diff,
- u32, mode, u64, flags)
+ u32, mode, u64, flags)
{
if (unlikely(flags))
return -EINVAL;
@@ -2542,6 +2542,125 @@ static const struct bpf_func_proto bpf_skb_adjust_room_proto = {
.arg4_type = ARG_ANYTHING,
};
+static int bpf_skb_mpls_net_grow(struct sk_buff *skb, int len_diff)
+{
+ u32 off = skb_mac_header_len(skb); /*LL_RESERVED_SPACE ?? */
+ int ret;
+
+ ret = skb_cow(skb, len_diff);
+ if (unlikely(ret < 0))
+ return ret;
+
+ skb_set_inner_protocol(skb, skb->protocol);
+ skb_reset_inner_network_header(skb);
+
+ ret = bpf_skb_generic_push(skb, off, len_diff);
+ if (unlikely(ret < 0))
+ return ret;
+
+ skb_reset_mac_header(skb);
+ skb_set_network_header(skb, ETH_HLEN);
+ skb->protocol = eth_hdr(skb)->h_proto = htons(ETH_P_MPLS_UC);
+
+ if (skb_is_gso(skb)) {
+/* Due to header grow, MSS needs to be downgraded. */
+ skb_shinfo(skb)->gso_size -= len_diff;
+/* Header must be checked, and gso_segs recomputed. */
+ skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
+ skb_shinfo(skb)->gso_segs = 0;
+ }
+
+ bpf_compute_data_pointers(skb);
+ return 0;
+}
+
+BPF_CALL_2(bpf_skb_mpls_pop, struct sk_buff*, skb, u32, num_lbls)
+{
+ u32 i = 0;
+ struct mpls_shim_hdr *hdr;
+ unsigned char *cursor;
+ struct mpls_entry_decoded dec;
+
+ if (num_lbls == 0)
+ return 0;
+
+ cursor = skb_network_header(skb);
+ do {
+ hdr = (struct mpls_shim_hdr *)cursor;
+ dec = mpls_entry_decode(hdr);
+ i++; cursor = cursor + sizeof(struct mpls_shim_hdr);
+ } while (dec.bos != 1 && i < num_lbls);
+
+ bpf_push_mac_rcsum(skb);
+ skb_pull(skb, i * sizeof(struct mpls_shim_hdr));
+ skb_reset_network_header(skb);
+
+ skb->protocol = eth_hdr(skb)->h_proto = htons(ETH_P_MPLS_UC);
+ bpf_pull_mac_rcsum(skb);
+ bpf_compute_data_pointers(skb);
+
+ return i;
+}
+
+const struct bpf_func_proto bpf_skb_mpls_pop_proto = {
+ .func = bpf_skb_mpls_pop,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_ANYTHING,
+};
+EXPORT_SYMBOL_GPL(bpf_skb_mpls_pop_proto);
+
+BPF_CALL_3(bpf_skb_mpls_push, struct sk_buff*, skb,
+ __be32*, lbls, u32, num_lbls)
+{
+ int ret, i;
+ unsigned int new_header_size = num_lbls * sizeof(__be32);
+ unsigned int ttl = 255;
+ struct dst_entry *dst = skb_dst(skb);
+ struct net_device *out_dev = dst->dev;
+ struct mpls_shim_hdr *hdr;
+ bool bos;
+
+ /* Ensure there is enough space for the headers in the skb */
+ ret = bpf_skb_mpls_net_grow(skb, new_header_size);
+ if (ret < 0) {
+ trace_printk("COW was killed\n");
+ bpf_compute_data_pointers(skb);
+ return -ENOMEM;
+ }
+
+ skb->dev = out_dev;
+/* XXX this may need finesse to integrate with
+ * global TTL values for MPLS
+ */
+ if (dst->ops->family == AF_INET)
+ ttl = ip_hdr(skb)->ttl;
+ else if (dst->ops->family == AF_INET6)
+ ttl = ipv6_hdr(skb)->hop_limit;
+
+ /* Push the new labels */
+ hdr = skb_mpls_hdr(skb);
+ bos = true;
+ for (i = num_lbls - 1; i >= 0; i--) {
+ hdr[i] = mpls_entry_encode(lbls[i], ttl, 0, bos);
+ bos = false;
+ }
+
+ bpf_compute_data_pointers(skb);
+ return 0;
+}
+
+const struct bpf_func_proto bpf_skb_mpls_push_proto = {
+ .func = bpf_skb_mpls_push,
+ .gpl_only = false,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_PTR_TO_MEM,
+ .arg3_type = ARG_CONST_SIZE,
+};
+EXPORT_SYMBOL_GPL(bpf_skb_mpls_push_proto);
+
static u32 __bpf_skb_min_len(const struct sk_buff *skb)
{
u32 min_len = skb_network_offset(skb);
@@ -3019,6 +3138,8 @@ bool bpf_helper_changes_pkt_data(void *func)
{
if (func == bpf_skb_vlan_push ||
func == bpf_skb_vlan_pop ||
+ func == bpf_skb_mpls_push ||
+ func == bpf_skb_mpls_pop ||
func == bpf_skb_store_bytes ||
func == bpf_skb_change_proto ||
func == bpf_skb_change_head ||
@@ -3682,6 +3803,10 @@ tc_cls_act_func_proto(enum bpf_func_id func_id)
return &bpf_skb_vlan_push_proto;
case BPF_FUNC_skb_vlan_pop:
return &bpf_skb_vlan_pop_proto;
+ case BPF_FUNC_skb_mpls_push:
+ return &bpf_skb_mpls_push_proto;
+ case BPF_FUNC_skb_mpls_pop:
+ return &bpf_skb_mpls_pop_proto;
case BPF_FUNC_skb_change_proto:
return &bpf_skb_change_proto_proto;
case BPF_FUNC_skb_change_type:
--
2.16.2.windows.1
^ permalink raw reply related
* Re: [PATCH v3 2/4] bus: fsl-mc: add restool userspace support
From: Andrew Lunn @ 2018-03-28 16:28 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ioana Ciornei, gregkh, Laurentiu Tudor, Linux Kernel Mailing List,
Stuart Yoder, Ruxandra Ioana Ciocoi Radulescu, Razvan Stefanescu,
Roy Pledge, Networking
In-Reply-To: <CAK8P3a1aQCm_r95Ka4x+j8SJLCLFvLuX3A4twK1WJcJs66YwKg@mail.gmail.com>
> 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.
Hi Arnd
I agree that switchdev and devlink should be the correct way to handle
this. The low level plumbing of the hardware should all be
hidden. There should not be any user space commands needed other than
the usual network configuration tools and devlink.
Andrew
^ permalink raw reply
* Re: net_tx_action race condition?
From: Eric Dumazet @ 2018-03-28 16:32 UTC (permalink / raw)
To: Saurabh Kr, Angelo Rizzi
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Sarvendra Vikram Singh, Kunal Sharma
In-Reply-To: <BY2PR05MB712796CB064F3202FEFA328B3A30@BY2PR05MB712.namprd05.prod.outlook.com>
On 03/28/2018 12:30 AM, Saurabh Kr wrote:
> Hi Eric/Angelo,
>
> We are seeing the assertion error in linux kernel 2.4.29 “*kernel: KERNEL: assertion (atomic_read(&skb->users) == 0) failed at dev.c(1397)**”.* Based on patch provided (_https://patchwork.kernel.org/patch/5368051/_ ) we merged the changes in linux kernel 2.4.29 but we are still facing the assertion error at dev.c (1397). Please let me know your thoughts.
>
> *Before Merge**(linux 2.4.29)*
> ---------------------------------
>
> static void net_tx_action(struct softirq_action *h)
> {
> int cpu = smp_processor_id();
>
> if (softnet_data[cpu].completion_queue) {
> struct sk_buff *clist;
>
> local_irq_disable();
> clist = softnet_data[cpu].completion_queue; // Existing code
> softnet_data[cpu].completion_queue = NULL;
> local_irq_enable();
>
> while (clist != NULL) {
> struct sk_buff *skb = clist;
> clist = clist->next;
>
> BUG_TRAP(atomic_read(&skb->users) == 0);
> __kfree_skb(skb);
> }
> }
>
> ---------
>
> *After Merge the changes based on available patch**(linux 2.4.29)**:*
> ------------------------------------------------------------------------------
>
> static void net_tx_action(struct softirq_action *h)
> {
> int cpu = smp_processor_id();
>
> if (softnet_data[cpu].completion_queue) {
> struct sk_buff *clist;
>
> local_irq_disable();
> clist = *(volatile typeof(softnet_data[cpu].completion_queue) *)&( softnet_data[cpu].completion_queue); // Modified line based on available patch
> softnet_data[cpu].completion_queue = NULL;
> local_irq_enable();
>
> while (clist != NULL) {
> struct sk_buff *skb = clist;
> clist = clist->next;
>
> BUG_TRAP(atomic_read(&skb->users) == 0);
> __kfree_skb(skb);
> }
> }
> ………….
>
> Thanks & regards,
> Saurabh
>
Thats simply prove (again) that this 'fix' was not the proper one.
I have no idea what is wrong, and there is no way I am going to look at 2.4.29 kernel...
^ permalink raw reply
* Re: 4.14.29 - tcp_push() - null skb's cb dereference
From: Eric Dumazet @ 2018-03-28 16:33 UTC (permalink / raw)
To: David Miller; +Cc: krzysztof.blaszkowski, netdev, gregkh
In-Reply-To: <20180328.103818.621024177280113028.davem@davemloft.net>
On 03/28/2018 07:38 AM, David Miller wrote:
> 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 a lot David.
^ 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