* Re: [PATCH net-next 1/2] xdp: Always use a devmap for XDP_REDIRECT to a device
From: Toke Høiland-Jørgensen @ 2019-02-26 11:00 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Jesper Dangaard Brouer, David Miller, netdev, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190225104757.75b622c9@cakuba.netronome.com>
Jakub Kicinski <jakub.kicinski@netronome.com> writes:
> On Sat, 23 Feb 2019 13:11:02 +0100, Toke Høiland-Jørgensen wrote:
>> Jesper Dangaard Brouer <brouer@redhat.com> writes:
>> > On Fri, 22 Feb 2019 13:37:34 -0800 Jakub Kicinski wrote:
>> >> On Fri, 22 Feb 2019 11:13:50 +0100, Toke Høiland-Jørgensen wrote:
>> >> > Jakub Kicinski <jakub.kicinski@netronome.com> writes:
>> >> > > On Thu, 21 Feb 2019 12:56:54 +0100, Toke Høiland-Jørgensen wrote:
>> > [...]
>> >> > >
>> >> > > BPF programs don't obey by netns boundaries. The fact the program is
>> >> > > verified in one ns doesn't mean this is the only ns it will be used in :(
>> >> > > Meaning if any program is using the redirect map you may need a secret
>> >> > > map in every ns.. no?
>> >> >
>> >> > Ah, yes, good point. Totally didn't think about the fact that load and
>> >> > attach are decoupled. Hmm, guess I'll just have to move the call to
>> >> > alloc_default_map() to the point where the program is attached to an
>> >> > interface, then...
>> >>
>> >> Possibly.. and you also need to handle the case where interface with a
>> >> program attached is moved, no?
>>
>> Yup, alloc on attach was easy enough; the moving turns out to be the
>> tricky part :)
>>
>> > True, we need to handle if e.g. a veth gets an XDP program attached and
>> > then is moved into a network namespace (as I've already explained to
>> > Toke in a meeting).
>>
>> Yeah, I had somehow convinced myself that the XDP program was being
>> removed when the interface was being torn down before moving between
>> namespaces. Jesper pointed out that this was not in fact the case... :P
>>
>> > I'm still not sure how to handle this...
>>
>> There are a couple of options, I think. At least:
>>
>> 1. Maintain a flag on struct net_device indicating that this device
>> needs the redirect map allocated, and react to that when interfaces
>> are being moved.
>>
>> 2. Lookup the BPF program by ID (which we can get from the driver) on
>> move, and react to the program flag.
>>
>> 3. Keep the allocation on program load, but allocate maps for all active
>> namespaces (which would probably need a refcnt mechanism to
>> deallocate things again).
>>
>> I think I'm leaning towards #2; possibly combined with a refcnt so we
>> can actually deallocate the map in the root namespace when it's not
>> needed anymore.
>
> Okay.. what about tail calls? I think #3 is most reasonable
> complexity- -wise, or some mix of #2 and #3 - cnt the programs with
> legacy redirects, and then allocate the resources if cnt && name space
> has any XDP program attached.
Yeah, I have that more or less working; except I forgot about tail
calls, but that should not be too difficult to fix.
> Can users really not be told to just use the correct helper? ;)
Experience would suggest not; users tend to use the simplest API that
gets their job done. And then wonder why they don't get the nice
performance numbers they were "promised". And, well, I tend to agree
that it's not terribly friendly to just go "use this other more
complicated API if you want proper performance". If we really mean that,
then we should formally deprecate xdp_redirect() as an API, IMO :)
-Toke
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Oliver Hartkopp @ 2019-02-26 11:04 UTC (permalink / raw)
To: Julius Niedworok, linux-wireless
Cc: ga58taw, david, nc, David S. Miller, Johannes Berg, Edward Cree,
Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
linux-kernel
In-Reply-To: <20190226094104.35192-1-julius.n@gmx.net>
Hi Julius,
On 26.02.19 10:40, Julius Niedworok wrote:
> In order to force delivery of TX status frames for research and debugging
> purposes, implement the IFF_ECHO flag for ieee80211 devices. When this flag
> is set for a specific interface, IEEE80211_TX_CTL_REQ_TX_STATUS is enabled
> in all packets sent from that interface. IFF_ECHO can be set via
> /sys/class/net/<dev>/flags. The default is disabled.
(..)
> + /*
> + * Force TX status frames on ieee80211 devices.
> + * Since IFF_ECHO is used by CAN devices for a different
> + * purpose, we must check dev->ieee80211_ptr.
> + */
The reason for IFF_ECHO was, that the data frame which is sent onto the
wire (by one application) is not visible to all the other applications
on the same (local) host. Therefore a successful transmission on the
wire triggers the 'echo' of the sent content into the local host.
So what are you getting back after you enabled IFF_ECHO on your mac80211
device?
Is it just a 'status' about a sent packet, or is it the packet ('full
content') itself?
Regards,
Oliver
^ permalink raw reply
* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Martin Blumenstingl @ 2019-02-26 11:37 UTC (permalink / raw)
To: Anand Moon; +Cc: Bartosz Golaszewski, linux-amlogic, netdev
In-Reply-To: <CANAwSgSw4dCQnHv0cMSxDs7S=83NUh5B6M8m_-ooFQKv02=v_g@mail.gmail.com>
Hi Anand,
On Tue, Feb 26, 2019 at 11:26 AM Anand Moon <linux.amoon@gmail.com> wrote:
>
> Hi Martin,
>
> On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > hi Martin,
> >
> > +Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> > <martin.blumenstingl@googlemail.com> wrote:
> > >
> > > I have seen Anand's your question in [0]:
> > > > only issue is I have is the each time their is random MAC address so I
> > > > get new IP from dhcp server.
> > > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> > >
> > > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > > pass it (via the .dtb) to the kernel.
> > > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > > for and example.
> > >
> > > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > > controllers: [2] (please note that the function implementing this
> > > binding was recently renamed: [3])
> > > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > > based binding yet
> > >
> > > Anand, if you want to work on this: feel free to do so!
> > > I have the SDHC MMC driver and a discussion about the power-domain
> > > drivers on my TODO-list, so I'm pretty busy at the moment.
> > >
> > >
> > > Regards
> > > Martin
> > >
> >
> > Thanks for your inputs :) 8)
> >
> > After enable CONFIG_MESON_MX_EFUSE and added the alias
> >
>
> As far as I can tell this nvmem consist of field.
> Board ID
> MAC address
> Serial Number
> UID
>
> I feel efues value in nvmem are read with following offset just for testing.
> but it also need some driver changes to read from secure memory
> by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> > On Odroid C1+
> > # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> > 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> > 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> > 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> > *
> > 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000 *00:1e:06:10:dc:ad*
> > mac address from nvmem
> > 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> > 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> > 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> > 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> > 0000200
> >
> arch/arm/boot/dts/meson8b.dtsi
> @@ -360,6 +360,18 @@
> compatible = "amlogic,meson8b-efuse";
> clocks = <&clkc CLKID_EFUSE>;
> clock-names = "core";
> +
> + board_sn: board-sn@0000000 {
> + reg = <0x0000000 0x10>;
> + };
> +
> + eth_mac: eth-mac@00001b4 {
> + reg = <0x00001b4 0x6>;
> + };
> +
> + board_sno: board-sno@00001c0 {
> + reg = <0x00001c0 0x30>;
> + };
>
> This is what I am looking into.
> If you have some input please share.
your findings are looking good to me!
I believe that some of the offsets are specific to the Odroid-C1.
this is what the Endless Mini EC-100's kernel uses: [0] (eth_mac
matches, but it seems that the other two don't)
Regards
Martin
[0] https://github.com/endlessm/linux-meson/blob/9969d20dc0f034e7a5addd4f6d28a8193e956b16/drivers/amlogic/efuse/efuse_version.c#L519
^ permalink raw reply
* [PATCH net 4/4] tls: Fix tls_device receive
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
Currently, the receive function fails to handle records already
decrypted by the device due to the commit mentioned below.
This commit advances the TLS record sequence number and prepares the context
to handle the next record.
Fixes: fedf201e1296 ("net: tls: Refactor control message handling on recv")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
net/tls/tls_sw.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index f515cd7e984e..85da10182d8d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1481,18 +1481,17 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
return err;
}
-
- rxm->full_len -= padding_length(ctx, tls_ctx, skb);
-
- rxm->offset += prot->prepend_size;
- rxm->full_len -= prot->overhead_size;
- tls_advance_record_sn(sk, &tls_ctx->rx, version);
- ctx->decrypted = true;
- ctx->saved_data_ready(sk);
} else {
*zc = false;
}
+ rxm->full_len -= padding_length(ctx, tls_ctx, skb);
+ rxm->offset += prot->prepend_size;
+ rxm->full_len -= prot->overhead_size;
+ tls_advance_record_sn(sk, &tls_ctx->rx, version);
+ ctx->decrypted = true;
+ ctx->saved_data_ready(sk);
+
return err;
}
--
2.12.2
^ permalink raw reply related
* [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
From: Eran Ben Elisha <eranbe@mellanox.com>
Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
application data TLS records. Moreover, it assumes that if the cipher
can be handled asynchronously, then all packets will be processed
asynchronously.
However, this assumption is not always true. Specifically, for AES-GCM
in TLS1.2, it causes data corruption, and breaks user applications.
This patch fixes this problem by separating the async capability from
the decryption operation result.
Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data records")
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Reviewed-by: Boris Pismenny <borisp@mellanox.com>
---
net/tls/tls_sw.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 4afa67b00aaf..f515cd7e984e 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
bool zc = false;
int to_decrypt;
int chunk = 0;
- bool async;
+ bool async_capable;
+ bool async = false;
skb = tls_wait_data(sk, psock, flags, timeo, &err);
if (!skb) {
@@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
/* Do not use async mode if record is non-data */
if (ctx->control == TLS_RECORD_TYPE_DATA)
- async = ctx->async_capable;
+ async_capable = ctx->async_capable;
else
- async = false;
+ async_capable = false;
err = decrypt_skb_update(sk, skb, &msg->msg_iter,
- &chunk, &zc, async);
+ &chunk, &zc, async_capable);
if (err < 0 && err != -EINPROGRESS) {
tls_err_abort(sk, EBADMSG);
goto recv_end;
}
- if (err == -EINPROGRESS)
+ if (err == -EINPROGRESS && async_capable) {
+ async = true;
num_async++;
- else if (prot->version == TLS_1_3_VERSION)
+ } else if (prot->version == TLS_1_3_VERSION) {
tlm->control = ctx->control;
+ }
/* If the type of records being processed is not known yet,
* set it to record type just dequeued. If it is already known,
--
2.12.2
^ permalink raw reply related
* [PATCH net 1/4] tls: Fix tls_device handling of partial records
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
Cleanup the handling of partial records while fixing a bug where the
tls_push_pending_closed_record function is using the software tls
context instead of the hardware context.
The bug resulted in the following crash:
[ 88.791229] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 88.793271] #PF error: [normal kernel read fault]
[ 88.794449] PGD 800000022a426067 P4D 800000022a426067 PUD 22a156067 PMD 0
[ 88.795958] Oops: 0000 [#1] SMP PTI
[ 88.796884] CPU: 2 PID: 4973 Comm: openssl Not tainted 5.0.0-rc4+ #3
[ 88.798314] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 88.800067] RIP: 0010:tls_tx_records+0xef/0x1d0 [tls]
[ 88.801256] Code: 00 02 48 89 43 08 e8 a0 0b 96 d9 48 89 df e8 48 dd
4d d9 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 88.805179] RSP: 0018:ffffbd888186fca8 EFLAGS: 00010213
[ 88.806458] RAX: ffff9af1ed657c98 RBX: ffff9af1e88a1980 RCX: 0000000000000000
[ 88.808050] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff9af1e88a1980
[ 88.809724] RBP: ffff9af1e88a1980 R08: 0000000000000017 R09: ffff9af1ebeeb700
[ 88.811294] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 88.812917] R13: ffff9af1e88a1980 R14: ffff9af1ec13f800 R15: 0000000000000000
[ 88.814506] FS: 00007fcad2240740(0000) GS:ffff9af1f7880000(0000) knlGS:0000000000000000
[ 88.816337] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 88.817717] CR2: 0000000000000000 CR3: 0000000228b3e000 CR4: 00000000001406e0
[ 88.819328] Call Trace:
[ 88.820123] tls_push_data+0x628/0x6a0 [tls]
[ 88.821283] ? remove_wait_queue+0x20/0x60
[ 88.822383] ? n_tty_read+0x683/0x910
[ 88.823363] tls_device_sendmsg+0x53/0xa0 [tls]
[ 88.824505] sock_sendmsg+0x36/0x50
[ 88.825492] sock_write_iter+0x87/0x100
[ 88.826521] __vfs_write+0x127/0x1b0
[ 88.827499] vfs_write+0xad/0x1b0
[ 88.828454] ksys_write+0x52/0xc0
[ 88.829378] do_syscall_64+0x5b/0x180
[ 88.830369] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 88.831603] RIP: 0033:0x7fcad1451680
[ 1248.470626] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 1248.472564] #PF error: [normal kernel read fault]
[ 1248.473790] PGD 0 P4D 0
[ 1248.474642] Oops: 0000 [#1] SMP PTI
[ 1248.475651] CPU: 3 PID: 7197 Comm: openssl Tainted: G OE 5.0.0-rc4+ #3
[ 1248.477426] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
[ 1248.479310] RIP: 0010:tls_tx_records+0x110/0x1f0 [tls]
[ 1248.480644] Code: 00 02 48 89 43 08 e8 4f cb 63 d7 48 89 df e8 f7 9c
1b d7 4c 89 f8 4d 8b bf 98 00 00 00 48 05 98 00 00 00 48 89 04 24 49 39
c7 <49> 8b 1f 4d 89 fd 0f 84 af 00 00 00 41 8b 47 10 85 c0 0f 85 8d 00
[ 1248.484825] RSP: 0018:ffffaa0a41543c08 EFLAGS: 00010213
[ 1248.486154] RAX: ffff955a2755dc98 RBX: ffff955a36031980 RCX: 0000000000000006
[ 1248.487855] RDX: 0000000000000000 RSI: 000000000000002b RDI: 0000000000000286
[ 1248.489524] RBP: ffff955a36031980 R08: 0000000000000000 R09: 00000000000002b1
[ 1248.491394] R10: 0000000000000003 R11: 00000000ad55ad55 R12: 0000000000000000
[ 1248.493162] R13: 0000000000000000 R14: ffff955a2abe6c00 R15: 0000000000000000
[ 1248.494923] FS: 0000000000000000(0000) GS:ffff955a378c0000(0000) knlGS:0000000000000000
[ 1248.496847] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1248.498357] CR2: 0000000000000000 CR3: 000000020c40e000 CR4: 00000000001406e0
[ 1248.500136] Call Trace:
[ 1248.500998] ? tcp_check_oom+0xd0/0xd0
[ 1248.502106] tls_sk_proto_close+0x127/0x1e0 [tls]
[ 1248.503411] inet_release+0x3c/0x60
[ 1248.504530] __sock_release+0x3d/0xb0
[ 1248.505611] sock_close+0x11/0x20
[ 1248.506612] __fput+0xb4/0x220
[ 1248.507559] task_work_run+0x88/0xa0
[ 1248.508617] do_exit+0x2cb/0xbc0
[ 1248.509597] ? core_sys_select+0x17a/0x280
[ 1248.510740] do_group_exit+0x39/0xb0
[ 1248.511789] get_signal+0x1d0/0x630
[ 1248.512823] do_signal+0x36/0x620
[ 1248.513822] exit_to_usermode_loop+0x5c/0xc6
[ 1248.515003] do_syscall_64+0x157/0x180
[ 1248.516094] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1248.517456] RIP: 0033:0x7fb398bd3f53
[ 1248.518537] Code: Bad RIP value.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 20 ++++----------------
net/tls/tls_device.c | 9 +++++----
net/tls/tls_main.c | 13 -------------
3 files changed, 9 insertions(+), 33 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 9f4117ae2297..a528a082da73 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -199,10 +199,6 @@ struct tls_offload_context_tx {
(ALIGN(sizeof(struct tls_offload_context_tx), sizeof(void *)) + \
TLS_DRIVER_STATE_SIZE)
-enum {
- TLS_PENDING_CLOSED_RECORD
-};
-
struct cipher_context {
char *iv;
char *rec_seq;
@@ -335,17 +331,14 @@ int tls_push_sg(struct sock *sk, struct tls_context *ctx,
int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
int flags);
-int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
- int flags, long *timeo);
-
static inline struct tls_msg *tls_msg(struct sk_buff *skb)
{
return (struct tls_msg *)strp_msg(skb);
}
-static inline bool tls_is_pending_closed_record(struct tls_context *ctx)
+static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
{
- return test_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
+ return !!ctx->partially_sent_record;
}
static inline int tls_complete_pending_work(struct sock *sk,
@@ -357,17 +350,12 @@ static inline int tls_complete_pending_work(struct sock *sk,
if (unlikely(sk->sk_write_pending))
rc = wait_on_pending_writer(sk, timeo);
- if (!rc && tls_is_pending_closed_record(ctx))
- rc = tls_push_pending_closed_record(sk, ctx, flags, timeo);
+ if (!rc && tls_is_partially_sent_record(ctx))
+ rc = tls_push_partial_record(sk, ctx, flags);
return rc;
}
-static inline bool tls_is_partially_sent_record(struct tls_context *ctx)
-{
- return !!ctx->partially_sent_record;
-}
-
static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
{
return tls_ctx->pending_open_record_frags;
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index a5c17c47d08a..3e5e8e021a87 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -271,7 +271,6 @@ static int tls_push_record(struct sock *sk,
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, ctx->crypto_send.info.version);
for (i = 0; i < record->num_frags; i++) {
@@ -368,9 +367,11 @@ static int tls_push_data(struct sock *sk,
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;
+ if (tls_is_partially_sent_record(tls_ctx)) {
+ rc = tls_push_partial_record(sk, tls_ctx, flags);
+ if (rc < 0)
+ return rc;
+ }
pfrag = sk_page_frag(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index caff15b2f9b2..7e05af75536d 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -209,19 +209,6 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
return tls_push_sg(sk, ctx, sg, offset, flags);
}
-int tls_push_pending_closed_record(struct sock *sk,
- struct tls_context *tls_ctx,
- int flags, long *timeo)
-{
- struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
-
- if (tls_is_partially_sent_record(tls_ctx) ||
- !list_empty(&ctx->tx_list))
- return tls_tx_records(sk, flags);
- else
- return tls_ctx->push_pending_record(sk, flags);
-}
-
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH net 2/4] tls: Fix write space handling
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
In-Reply-To: <20190226121235.20784-1-borisp@mellanox.com>
TLS device cannot use the sw context. This patch returns the original
tls device write space handler and moves the sw/device specific portions
to the relevant files.
Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
---
include/net/tls.h | 3 +++
net/tls/tls_device.c | 16 ++++++++++++++++
net/tls/tls_main.c | 17 +++++++++--------
net/tls/tls_sw.c | 15 +++++++++++++++
4 files changed, 43 insertions(+), 8 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index a528a082da73..9d7c53737b13 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock *sk)
return !!tls_sw_ctx_tx(ctx);
}
+int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
+int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
+
static inline struct tls_offload_context_rx *
tls_offload_ctx_rx(const struct tls_context *tls_ctx)
{
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index 3e5e8e021a87..e8988b3f3236 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct sock *sk, int flags)
return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
}
+int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ int rc = 0;
+
+ if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
+ gfp_t sk_allocation = sk->sk_allocation;
+
+ sk->sk_allocation = GFP_ATOMIC;
+ rc = tls_push_partial_record(sk, ctx,
+ MSG_DONTWAIT | MSG_NOSIGNAL);
+ sk->sk_allocation = sk_allocation;
+ }
+
+ return rc;
+}
+
void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 7e05af75536d..11c1980a75cb 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
static void tls_write_space(struct sock *sk)
{
struct tls_context *ctx = tls_get_ctx(sk);
- struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
+ int rc;
/* If in_tcp_sendpages call lower protocol write space handler
* to ensure we wake up any waiting operations there. For example
@@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
return;
}
- /* Schedule the transmission if tx list is ready */
- if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
- /* Schedule the transmission */
- if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx->tx_bitmask))
- schedule_delayed_work(&tx_ctx->tx_work.work, 0);
- }
+#ifdef CONFIG_TLS_DEVICE
+ if (ctx->tx_conf == TLS_HW)
+ rc = tls_device_write_space(sk, ctx);
+ else
+#endif
+ rc = tls_sw_write_space(sk, ctx);
- ctx->sk_write_space(sk);
+ if (!rc)
+ ctx->sk_write_space(sk);
}
static void tls_ctx_free(struct tls_context *ctx)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 1cc830582fa8..4afa67b00aaf 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct *work)
release_sock(sk);
}
+int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
+{
+ struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
+
+ /* Schedule the transmission if tx list is ready */
+ if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
+ /* Schedule the transmission */
+ if (!test_and_set_bit(BIT_TX_SCHEDULED,
+ &tx_ctx->tx_bitmask))
+ schedule_delayed_work(&tx_ctx->tx_work.work, 0);
+ }
+
+ return 0;
+}
+
int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
--
2.12.2
^ permalink raw reply related
* [PATCH net 0/4] tls: Fix issues in tls_device
From: Boris Pismenny @ 2019-02-26 12:12 UTC (permalink / raw)
To: aviadye, davejwatson, john.fastabend, daniel, vakul.garg, netdev
Cc: eranbe, borisp
This series fixes issues encountered in tls_device code paths,
which were introduced recently.
Additionally, this series includes a fix for tls software only receive flow,
which causes corruption of payload received by user space applications.
This series was tested using the OpenSSL integration of KTLS -
https://github.com/mellanox/openssl/tree/tls_rx3
Boris Pismenny (3):
tls: Fix tls_device handling of partial records
tls: Fix write space handling
tls: Fix tls_device receive
Eran Ben Elisha (1):
tls: Fix mixing between async capable and async
include/net/tls.h | 23 +++++++----------------
net/tls/tls_device.c | 25 +++++++++++++++++++++----
net/tls/tls_main.c | 30 +++++++++---------------------
net/tls/tls_sw.c | 45 +++++++++++++++++++++++++++++++--------------
4 files changed, 68 insertions(+), 55 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: 32-bit Amlogic SoCs: avoid using Ethernet MAC addresses
From: Anand Moon @ 2019-02-26 12:13 UTC (permalink / raw)
To: Martin Blumenstingl; +Cc: Bartosz Golaszewski, linux-amlogic, netdev
In-Reply-To: <CAFBinCCCnCBO4dbvGjDuiH6Ba-9mcg3Ax3-vkLipGijubNBFBA@mail.gmail.com>
Hi Martin,
On Tue, 26 Feb 2019 at 17:07, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Tue, Feb 26, 2019 at 11:26 AM Anand Moon <linux.amoon@gmail.com> wrote:
> >
> > Hi Martin,
> >
> > On Mon, 25 Feb 2019 at 17:49, Anand Moon <linux.amoon@gmail.com> wrote:
> > >
> > > hi Martin,
> > >
> > > +Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > >
> > > On Mon, 25 Feb 2019 at 02:25, Martin Blumenstingl
> > > <martin.blumenstingl@googlemail.com> wrote:
> > > >
> > > > I have seen Anand's your question in [0]:
> > > > > only issue is I have is the each time their is random MAC address so I
> > > > > get new IP from dhcp server.
> > > > > How can I avoid this. I have tried to enable eFuse driver but with no success.
> > > >
> > > > u-boot on the 64-bit SoCs can read the MAC address from the eFuse and
> > > > pass it (via the .dtb) to the kernel.
> > > > This requires an ethernet0 alias in the mainline .dts though, see [1]
> > > > for and example.
> > > >
> > > > I'm not sure if this also works with the older u-boot on the 32-bit SoCs.
> > > > if it doesn't then there's a nvmem-cells binding for all Ethernet
> > > > controllers: [2] (please note that the function implementing this
> > > > binding was recently renamed: [3])
> > > > as far as I can tell the stmmac driver doesn't support the nvmem-cells
> > > > based binding yet
> > > >
> > > > Anand, if you want to work on this: feel free to do so!
> > > > I have the SDHC MMC driver and a discussion about the power-domain
> > > > drivers on my TODO-list, so I'm pretty busy at the moment.
> > > >
> > > >
> > > > Regards
> > > > Martin
> > > >
> > >
> > > Thanks for your inputs :) 8)
> > >
> > > After enable CONFIG_MESON_MX_EFUSE and added the alias
> > >
> >
> > As far as I can tell this nvmem consist of field.
> > Board ID
> > MAC address
> > Serial Number
> > UID
> >
> > I feel efues value in nvmem are read with following offset just for testing.
> > but it also need some driver changes to read from secure memory
> > by enable CONFIG_MESON_SM and get the mac address to be set in ethernet driver.
> > > On Odroid C1+
> > > # hexdump /sys/devices/platform/soc/da000000.secbus/da000000.nvmem/meson8b-efuse0/nvmem
> > > 0000000 1143 0000 4b48 3143 3131 3232 3346 4537
> > > 0000010 3942 4432 0000 0000 0000 0000 0000 0000
> > > 0000020 0000 0000 0000 0000 0000 0000 0000 0000
> > > *
> > > 00001b0 0000 0000 1e00 1006 addc 0000 0000 0000 *00:1e:06:10:dc:ad*
> > > mac address from nvmem
> > > 00001c0 0000 0000 0000 4b48 3143 3133 3631 3131
> > > 00001d0 3732 3130 6237 6537 3165 6437 3034 3764
> > > 00001e0 02ad ec24 ff7f acfb d692 5300 0047 0000
> > > 00001f0 0000 0000 0000 aff6 a000 0000 1400 c100
> > > 0000200
> > >
> > arch/arm/boot/dts/meson8b.dtsi
> > @@ -360,6 +360,18 @@
> > compatible = "amlogic,meson8b-efuse";
> > clocks = <&clkc CLKID_EFUSE>;
> > clock-names = "core";
> > +
> > + board_sn: board-sn@0000000 {
> > + reg = <0x0000000 0x10>;
> > + };
> > +
> > + eth_mac: eth-mac@00001b4 {
> > + reg = <0x00001b4 0x6>;
> > + };
> > +
> > + board_sno: board-sno@00001c0 {
> > + reg = <0x00001c0 0x30>;
> > + };
> >
> > This is what I am looking into.
> > If you have some input please share.
> your findings are looking good to me!
>
> I believe that some of the offsets are specific to the Odroid-C1.
> this is what the Endless Mini EC-100's kernel uses: [0] (eth_mac
> matches, but it seems that the other two don't)
>
>
.title = "mac", //for the main network interface
.id = EFUSE_MAC_ID,
.offset = 436,
.enc_len = 6,
.data_len = 6,
offset for mac which is in dec *436* is equal to hex *0x1b4* which I
am using for testing now.
I am changing this offset setting so that I get consistence result out of it.
I will also check with armbina image and see if I can find some thing usefull.
Thanks for your valuable inputs once again :)
Best Regards
-Anand
>
> [0] https://github.com/endlessm/linux-meson/blob/9969d20dc0f034e7a5addd4f6d28a8193e956b16/drivers/amlogic/efuse/efuse_version.c#L519
^ permalink raw reply
* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Neil Horman @ 2019-02-26 12:29 UTC (permalink / raw)
To: Xin Long; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <CADvbK_dtfZnKQvWO5CMaVfaUg+yZUGH3TVN2xkWrqz7L+Z_MGQ@mail.gmail.com>
On Tue, Feb 26, 2019 at 12:15:54AM +0800, Xin Long wrote:
> On Mon, Feb 25, 2019 at 10:08 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > On Mon, Feb 25, 2019 at 09:20:44PM +0800, Xin Long wrote:
> > > On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > > >
> > > > On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > > > > sctp_hdr(skb) only works when skb->transport_header is set properly.
> > > > >
> > > > > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> > > > >
> > > > > skb->transport_header is not guaranteed to be right value for sctp.
> > > > > It will cause to fail to check the checksum for sctp packets.
> > > > >
> > > > > So fix it by using offset, which is always right in all places.
> > > > >
> > > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > > ---
> > > > > include/net/sctp/checksum.h | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > > > > index 32ee65a..1c6e6c0 100644
> > > > > --- a/include/net/sctp/checksum.h
> > > > > +++ b/include/net/sctp/checksum.h
> > > > > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> > > > > static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> > > > > unsigned int offset)
> > > > > {
> > > > > - struct sctphdr *sh = sctp_hdr(skb);
> > > > > + struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> > > > > const struct skb_checksum_ops ops = {
> > > > > .update = sctp_csum_update,
> > > > > .combine = sctp_csum_combine,
> > > > > --
> > > > > 2.1.0
> > > > >
> > > > >
> > > > Shouldn't you use skb_set_transport_header and skb_transport_header here?
> > > you mean:
> > > skb_set_transport_header(skb, offset);
> > > sh = sctp_hdr(skb);
> > > ?
> > >
> > > There's no place counting on here to set transport_header.
> > > It will be a kinda redundant job, yet skb is 'const'.
> > >
> > I'm not sure what you mean by "theres no place counting here". We have the
> > transport header offset, and you're doing the exact same computation that that
> > function does. It seems like we should use it in case the underlying
> > implementation changes.
> 1. skb_set_transport_header() and sctp_hdr() are like:
> skb->transport_header = skb->data - skb->head;
> skb->transport_header += offset
> sh = skb->head + skb->transport_header;
>
> 2. in this patch:
> sh = (struct sctphdr *)(skb->data + offset); only
>
> I think the 2nd one is better.
>
> I feel it's weird to set transport_header here if it's only for
> sctp_hdr(skb) in here.
>
> As for "underlying implementation changes", I don't know exactly the case
> but there are quite a few places doing things like:
> *hdr = (struct *hdr *)(skb->data + hdroff);
>
> I'd think it's safe. no?
>
Safe, yes, it just doesn't seem right. I know you've pointed out several places
below that rapidly compute transport offsets in a one-off fashion, but at this
same time, the other primary transports (tcp and udp), all seems to use the
transport header to do their work (linearizing as necessecary, which sctp also
does in sctp_rcv, at least in most cases).
> >
> > I understand what you are saying regarding the use of a const variable there,
> > but perhaps thats an argument for removing the const storage classifier. Better
> > still, it would be good to figure out why all paths to this function don't
> > already set the transport header offset to begin with (addressing your redundant
> > comment)
> The issue was reported when going to nf_conntrack by br_netfilter's
> bridge-nf-call-iptables.
> As you can see on nf_conntrack_in() path, even iphdr is got by:
> iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
> It's impossible to set skb->transport_header when we're not sure iphdr
> in linearized memory.
>
But if the skb isn't linearized, computing the transport header manually isn't
going to help you anyway. You can see that in skb_header_pointer. If the
offset they are trying to get to is outside the bounds of the length of the skb
(i.e. the fragmented case), it calls skb_copy_bits to linearize the needed
segment. It seems we should be doing something simmilar. In most cases we are
already linearized from sctp_rcv (possibly all, I need to think about that). All
I'm really saying is that by using the skb apis we insulate ourselves from
potential changes in how skbs might work in the future. I'm not strictly bound
to setting the transport header, but we should definately be getting the
transport header via the skb utility functions wherever possible.
Neil
^ permalink raw reply
* RE: [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Vakul Garg @ 2019-02-26 12:38 UTC (permalink / raw)
To: Boris Pismenny, aviadye@mellanox.com, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: eranbe@mellanox.com
In-Reply-To: <20190226121235.20784-4-borisp@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Tuesday, February 26, 2019 5:43 PM
> To: aviadye@mellanox.com; davejwatson@fb.com;
> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
> <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: eranbe@mellanox.com; borisp@mellanox.com
> Subject: [PATCH net 3/4] tls: Fix mixing between async capable and async
>
> From: Eran Ben Elisha <eranbe@mellanox.com>
>
> Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
> application data TLS records. Moreover, it assumes that if the cipher can be
> handled asynchronously, then all packets will be processed asynchronously.
>
> However, this assumption is not always true.
Could you please elaborate, what happens?
> Specifically, for AES-GCM in
> TLS1.2, it causes data corruption, and breaks user applications.
>
> This patch fixes this problem by separating the async capability from the
> decryption operation result.
>
> Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data
> records")
> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
> Reviewed-by: Boris Pismenny <borisp@mellanox.com>
> ---
> net/tls/tls_sw.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
> 4afa67b00aaf..f515cd7e984e 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
> bool zc = false;
> int to_decrypt;
> int chunk = 0;
> - bool async;
> + bool async_capable;
> + bool async = false;
>
> skb = tls_wait_data(sk, psock, flags, timeo, &err);
> if (!skb) {
> @@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
>
> /* Do not use async mode if record is non-data */
> if (ctx->control == TLS_RECORD_TYPE_DATA)
> - async = ctx->async_capable;
> + async_capable = ctx->async_capable;
> else
> - async = false;
> + async_capable = false;
>
> err = decrypt_skb_update(sk, skb, &msg->msg_iter,
> - &chunk, &zc, async);
> + &chunk, &zc, async_capable);
> if (err < 0 && err != -EINPROGRESS) {
> tls_err_abort(sk, EBADMSG);
> goto recv_end;
> }
>
> - if (err == -EINPROGRESS)
> + if (err == -EINPROGRESS && async_capable) {
Why do we need to check 'async_capable'?
Do we get err == -EINPROGRESS even when async_capable is false?
> + async = true;
> num_async++;
> - else if (prot->version == TLS_1_3_VERSION)
> + } else if (prot->version == TLS_1_3_VERSION) {
> tlm->control = ctx->control;
> + }
>
> /* If the type of records being processed is not known yet,
> * set it to record type just dequeued. If it is already known,
> --
> 2.12.2
^ permalink raw reply
* RE: [PATCH net 2/4] tls: Fix write space handling
From: Vakul Garg @ 2019-02-26 12:49 UTC (permalink / raw)
To: Boris Pismenny, aviadye@mellanox.com, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: eranbe@mellanox.com
In-Reply-To: <20190226121235.20784-3-borisp@mellanox.com>
> -----Original Message-----
> From: Boris Pismenny <borisp@mellanox.com>
> Sent: Tuesday, February 26, 2019 5:43 PM
> To: aviadye@mellanox.com; davejwatson@fb.com;
> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
> <vakul.garg@nxp.com>; netdev@vger.kernel.org
> Cc: eranbe@mellanox.com; borisp@mellanox.com
> Subject: [PATCH net 2/4] tls: Fix write space handling
>
> TLS device cannot use the sw context. This patch returns the original
> tls device write space handler and moves the sw/device specific portions
> to the relevant files.
>
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records
> for performance")
> Signed-off-by: Boris Pismenny <borisp@mellanox.com>
> Reviewed-by: Eran Ben Elisha <eranbe@mellanox.com>
> ---
> include/net/tls.h | 3 +++
> net/tls/tls_device.c | 16 ++++++++++++++++
> net/tls/tls_main.c | 17 +++++++++--------
> net/tls/tls_sw.c | 15 +++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index a528a082da73..9d7c53737b13 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -519,6 +519,9 @@ static inline bool tls_sw_has_ctx_tx(const struct sock
> *sk)
> return !!tls_sw_ctx_tx(ctx);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx);
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx);
> +
> static inline struct tls_offload_context_rx *
> tls_offload_ctx_rx(const struct tls_context *tls_ctx)
> {
> diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
> index 3e5e8e021a87..e8988b3f3236 100644
> --- a/net/tls/tls_device.c
> +++ b/net/tls/tls_device.c
> @@ -546,6 +546,22 @@ static int tls_device_push_pending_record(struct
> sock *sk, int flags)
> return tls_push_data(sk, &msg_iter, 0, flags,
> TLS_RECORD_TYPE_DATA);
> }
>
> +int tls_device_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + int rc = 0;
> +
> + if (!sk->sk_write_pending && tls_is_partially_sent_record(ctx)) {
> + gfp_t sk_allocation = sk->sk_allocation;
> +
> + sk->sk_allocation = GFP_ATOMIC;
> + rc = tls_push_partial_record(sk, ctx,
> + MSG_DONTWAIT |
> MSG_NOSIGNAL);
> + sk->sk_allocation = sk_allocation;
> + }
> +
> + return rc;
> +}
> +
> void handle_device_resync(struct sock *sk, u32 seq, u64 rcd_sn)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
> index 7e05af75536d..11c1980a75cb 100644
> --- a/net/tls/tls_main.c
> +++ b/net/tls/tls_main.c
> @@ -212,7 +212,7 @@ int tls_push_partial_record(struct sock *sk, struct
> tls_context *ctx,
> static void tls_write_space(struct sock *sk)
> {
> struct tls_context *ctx = tls_get_ctx(sk);
> - struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> + int rc;
>
> /* If in_tcp_sendpages call lower protocol write space handler
> * to ensure we wake up any waiting operations there. For example
> @@ -223,14 +223,15 @@ static void tls_write_space(struct sock *sk)
> return;
> }
>
> - /* Schedule the transmission if tx list is ready */
> - if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> - /* Schedule the transmission */
> - if (!test_and_set_bit(BIT_TX_SCHEDULED, &tx_ctx-
> >tx_bitmask))
> - schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> - }
> +#ifdef CONFIG_TLS_DEVICE
> + if (ctx->tx_conf == TLS_HW)
> + rc = tls_device_write_space(sk, ctx);
> + else
> +#endif
> + rc = tls_sw_write_space(sk, ctx);
>
> - ctx->sk_write_space(sk);
> + if (!rc)
Why do we need to check 'rc'?
If it is required, then ' ctx->sk_write_space(sk)' can move to tls_device_write_space()
since tls_sw_write_space() always returns '0'.
> + ctx->sk_write_space(sk);
> }
>
> static void tls_ctx_free(struct tls_context *ctx)
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 1cc830582fa8..4afa67b00aaf 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2126,6 +2126,21 @@ static void tx_work_handler(struct work_struct
> *work)
> release_sock(sk);
> }
>
> +int tls_sw_write_space(struct sock *sk, struct tls_context *ctx)
> +{
> + struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
> +
> + /* Schedule the transmission if tx list is ready */
> + if (is_tx_ready(tx_ctx) && !sk->sk_write_pending) {
> + /* Schedule the transmission */
> + if (!test_and_set_bit(BIT_TX_SCHEDULED,
> + &tx_ctx->tx_bitmask))
> + schedule_delayed_work(&tx_ctx->tx_work.work, 0);
> + }
> +
> + return 0;
> +}
> +
> int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
> {
> struct tls_context *tls_ctx = tls_get_ctx(sk);
> --
> 2.12.2
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Sheng Lan @ 2019-02-26 13:02 UTC (permalink / raw)
To: Stephen Hemminger
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <20190225080147.30128f73@shemminger-XPS-13-9360>
> On Mon, 25 Feb 2019 22:49:39 +0800
> Sheng Lan <lansheng@huawei.com> wrote:
>
>> From: Sheng Lan <lansheng@huawei.com>
>> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
>>
>> It can be reproduced by following steps:
>> 1. virtio_net NIC is configured with gso/tso on
>> 2. configure nginx as http server with an index file bigger than 1M bytes
>> 3. use tc netem to produce duplicate packets and delay:
>> tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
>> 4. continually curl the nginx http server to get index file on client
>> 5. BUG_ON is seen quickly
>>
>> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
>> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
>> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G W 5.0.0-rc6 #2
>> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
>> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
>> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
>> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
>> [10258690.372094] FS: 0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
>> [10258690.372094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
>> [10258690.372094] Call Trace:
>> [10258690.372094] <IRQ>
>> [10258690.372094] skb_to_sgvec+0x11/0x40
>> [10258690.372094] start_xmit+0x38c/0x520 [virtio_net]
>> [10258690.372094] dev_hard_start_xmit+0x9b/0x200
>> [10258690.372094] sch_direct_xmit+0xff/0x260
>> [10258690.372094] __qdisc_run+0x15e/0x4e0
>> [10258690.372094] net_tx_action+0x137/0x210
>> [10258690.372094] __do_softirq+0xd6/0x2a9
>> [10258690.372094] irq_exit+0xde/0xf0
>> [10258690.372094] smp_apic_timer_interrupt+0x74/0x140
>> [10258690.372094] apic_timer_interrupt+0xf/0x20
>> [10258690.372094] </IRQ>
>>
>> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
>> linear data size and nonlinear data size, thus BUG_ON triggered. The
>> bad skb's nonlinear data size is less than skb->data_len, because the
>> skb is cloned and a part of related cloned skb's nonlinear data is
>> split off.
>>
>> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
>> some time in qdisc. Due to the delay time, the original skb will be pushed
>> again later in __tcp_push_pending_frames when tcp receives new packets.
>> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
>> the original skb will be fragmented and the skb's nonlinear data will be
>> split off. The length of the skb cloned by netem will not be updated.
>> When we use virtio_net NIC, the duplicated cloned skb will be filled into
>> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
>>
>> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
>> the duplicated skb's nonlinear data is independent.
>>
>> Signed-off-by: Sheng Lan <lansheng@huawei.com>
>> Reported-by: Qin Ji <jiqin.ji@huawei.com>
>>
>> Fixes: 0afb51e7 ("netem: reinsert for duplication")
>
> This sounds like a bug in the other layers (either TCP or Virtio net)
> not handling a cloned skb properly.
>
I have traced the route of skb by printk, let me take an example to describe the problem to make it clearly:
Mss value equals to 1448. Limit value is the split size when tcp do tso_fragment, is depending on the size of the sending congestion window and mss value.
TCP layer transmit the index file to client, the original skb1 size is large:
...
tcp_write_xmit (skb1->data_len == 62264, limit == 2*mss == 2896)
tso_fragment (it needs to be fragmented by limit value)
skb_split (after split, skb1->data_len == 2896, skb_shinfo(skb1)->frags[0] == 2896, skb_shinfo(skb1)->nr_frags == 1)
...
netem_enqueue (netem construct a duplicate packet of skb1 by skb_clone)
skb2 = skb_clone(skb1) (skb1->data_len == skb2->data_len == 2896, skb1 and skb2 share the nonlinear data frags[0] == 2896)
waiting 30ms (skb1 and skb2 will be delayed in qdisc queue due to the netem delay configuration)
TCP layer receives new packets and trys to retransmit the skb1:
tcp_rcv_established
__tcp_push_pending_frames
tcp_write_xmit (skb1->data_len == 2896, cwnd size decreased or packets in flight increased, cause the limit decreased to 1*mss == 1448)
tso_fragment (limit value is less than skb1->data_len, skb1 will be fragmented again)
skb_split (the second time split, skb1 is cloned now and share nonlinear data with skb2.
skb1->data_len == 1448, frags[0] == 1448, a part of shared nonlinear data has been split off)
Now we can see:
skb1->data_len == 1448
skb2->data_len == 2896 (2896 is wrong value)
frags[0] == 1448
The route of skb2:
netem_enqueue (netem construct a duplicate packet, skb2 = skb_clone(skb1), put skb2 into queue)
waiting 30ms (delayed packet)
...
netem_dequeue (skb2->data_len == 2896, frags[0] == 1448)
sch_direct_xmit
dev_hard_start_xmit
xmit_one
start_xmit [virtio_net driver]
skb_to_sgvec (BUG_ON here )
The key is that skb be split by skb_split in tso_fragment again after netem clone it and share nonlinear data with another skb.
Processing of TCP seems OK, which push and fragment delayed packets in write queue. And virtio_net is the trigger of the BUG_ON.
So I replaced skb_clone with skb_copy in netem_enqueue, and the method worked. Currently, I have no better idea to fix it,
would you give me some inspiring advice ? If I am wrong, please correct me.
Thanks
^ permalink raw reply
* Re: [PATCH 2/2] ath9K: debugfs: Fix SPUR-DOWN field
From: Kalle Valo @ 2019-02-26 13:08 UTC (permalink / raw)
To: Andrea Greco
Cc: johannes.berg, Andrea Greco, QCA ath9k Development,
David S. Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190221231257.6221-1-andrea.greco.gapmilano@gmail.com>
Andrea Greco <a.greco@4sigma.it> wrote:
> SPUR DOWN field returns spurup instead of spurdown.
>
> Signed-off-by: Andrea Greco <a.greco@4sigma.it>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
d0480d4326e2 ath9k: debugfs: Fix SPUR-DOWN field
--
https://patchwork.kernel.org/patch/10824837/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath9k: remove set but not used variable 'acq'
From: Kalle Valo @ 2019-02-26 13:09 UTC (permalink / raw)
To: YueHaibing
Cc: QCA ath9k Development, YueHaibing, linux-wireless, netdev,
kernel-janitors
In-Reply-To: <20190225033246.127410-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/ath/ath9k/recv.c: In function 'ath_rx_count_airtime':
> drivers/net/wireless/ath/ath9k/recv.c:1010:18: warning:
> variable 'acq' set but not used [-Wunused-but-set-variable]
>
> It's not used after 89cea7493a34 ("ath9k: Switch to mac80211 TXQ scheduling
> and airtime APIs"). Also remove related variables.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Acked-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
03af21d6ba35 ath9k: remove set but not used variable 'acq'
--
https://patchwork.kernel.org/patch/10828153/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Julius Niedworok @ 2019-02-26 13:13 UTC (permalink / raw)
To: Oliver Hartkopp
Cc: linux-wireless, ga58taw, David Hildenbrand, nc, David S. Miller,
Johannes Berg, Edward Cree, Jiri Pirko, Ido Schimmel,
Petr Machata, Kirill Tkhai, Alexander Duyck, Amritha Nambiar,
Li RongQing, netdev, linux-kernel
In-Reply-To: <c522f2e0-1bef-8679-45f7-707854918820@hartkopp.net>
Hi Oliver,
> On 26.02.2019 12:04, Oliver Hartkopp wrote:
>
> Hi Julius,
>
(..)
>
> The reason for IFF_ECHO was, that the data frame which is sent onto the wire (by one application) is not visible to all the other applications on the same (local) host. Therefore a successful transmission on the wire triggers the 'echo' of the sent content into the local host.
>
Thank you for the explanation - I can adjust the comment, if you like to.
> So what are you getting back after you enabled IFF_ECHO on your mac80211 device?
>
> Is it just a 'status' about a sent packet, or is it the packet ('full content') itself?
We are actually getting back the full content of the packet. So it matches the behaviour of the 'echo' in CAN.
>
> Regards,
> Oliver
>
Many thanks,
Julius
^ permalink raw reply
* Re: [PATCH RFC] mac80211: Use IFF_ECHO to force delivery of tx_status frames
From: Johannes Berg @ 2019-02-26 13:33 UTC (permalink / raw)
To: Julius Niedworok, Oliver Hartkopp
Cc: linux-wireless, ga58taw, David Hildenbrand, nc, David S. Miller,
Edward Cree, Jiri Pirko, Ido Schimmel, Petr Machata, Kirill Tkhai,
Alexander Duyck, Amritha Nambiar, Li RongQing, netdev,
linux-kernel
In-Reply-To: <6A213CBE-0B6A-466A-B721-E6A728D4888D@gmx.net>
On Tue, 2019-02-26 at 14:13 +0100, Julius Niedworok wrote:
>
> Thank you for the explanation - I can adjust the comment, if you like to.
>
> > So what are you getting back after you enabled IFF_ECHO on your mac80211 device?
> >
> > Is it just a 'status' about a sent packet, or is it the packet ('full content') itself?
>
> We are actually getting back the full content of the packet. So it
> matches the behaviour of the 'echo' in CAN.
I don't think it does, really.
In CAN, if I understand correctly, this is used for regular operation
interfaces, where you might want to run 'tcpdump', on wifi the
equivalent would be 'tcpdump -i wlan0'. This *already* implements full
visibility of outgoing and incoming frames.
Not sure how CAN even manages *not to*, but I don't really need to care
:-)
You're proposing to add this to the *monitor* interfaces and you really
should have made the flag conditional on that to make that clear.
However, even on monitor interfaces, you typically *already* see the
frames you transmitted there (as raw frames, which is the only thing you
can do).
What you're proposing is to use IFF_ECHO to show frames transmitted
through *other* interfaces on the monitor interface.
I don't think the IFF_ECHO semantics really match this.
Additionally, drivers are sort of free to ignore the REQ_TX_STATUS, or
we could in the future add ways of using the _noskb to feed back TX
status to the state machines where needed, so I'm not really sure I even
_want_ this to be set in stone in such an API.
Now, I can also see how this can be useful for debugging, but it feels
to me like this should be a driver (debug) option?
johannes
^ permalink raw reply
* Re: Yet another approach for implementing connection tracking offload
From: Yossi Kuperman @ 2019-02-26 13:35 UTC (permalink / raw)
To: Marcelo Leitner
Cc: Guy Shattah, Aaron Conole, John Hurley, Simon Horman,
Justin Pettit, Gregory Rose, Eelco Chaudron, Flavio Leitner,
Florian Westphal, Jiri Pirko, Rashid Khan, Sushil Kulkarni,
Andy Gospodarek, Roi Dayan, Or Gerlitz, Rony Efraim,
davem@davemloft.net, Paul Blakey, netdev@vger.kernel.org,
Oz Shlomo
In-Reply-To: <20190222222354.GV10660@localhost.localdomain>
On 23/02/2019 0:23, Marcelo Leitner wrote:
> On Mon, Feb 18, 2019 at 07:00:19PM +0000, Yossi Kuperman wrote:
>> Hello All,
>>
>> Following is a description of yet another possible approach to
>> implement connection tracking offload. We would like to hear your
>> opinion. There is the “native” way of implementing such an offload
>> by mirroring the software tables to hardware. This way seems
>> straightforward and simple, but real life is much more complicated
>> than that. Alternatively, we can merge the data-path flows
>> (separated by recirc_id) and offload a single flow to hardware.
>>
>> The general idea is quite simple. When OVS-daemon configures TC with
>> a filter that recirculate, the driver merely pretends to offload it
>> and return success. Upon packet arrival (in software) we let it
> This has potential to be a support nightmare: things that should and
> seems to be but are actually not and expected not to be.
> IOW, not_in_hw should still be there somehow.
Agree, we should handle this properly.
Please note we have a similar behavior with MT. From TC perspective it
may seem that everything is offloaded (in_hw), but we will encounter a
miss for every new connection.
>> traverse TC as usual, except for now we notify the driver on each
>> successful match. By doing this, the driver has all the necessary
>> information to merge the participating flows---including connection
>> tracking 5-tuple---into one equivalent flow. We do such a merge and
>> offload only if the connection is established. Note: the same
>> mechanism to communicate a 5-tuple to the driver can be used to
>> notify on a filter match.
>>
>> It is the driver responsibility to build and maintain the list of
>> filters a (specific) packet hit along the TC walk. Once we reach the
> I'm assuming this could be a shared code amongst the drivers, like
> DIM. We really don't want different algorithms for this.
> With that, we would be using the driver as just a temporary storage,
> for the matches/actions. Maybe we can do that with skb extensions?
>
> Like, turn on a flight recorder extention and have one especial last
> tc action (or even embed it into core tc) to process it when the
> traversing finishes. And only then call the driver to update whatever
> is needed.
>
It is a possibility.
>> last filter (a terminating one, e.g., forward) the driver posts a
>> work on a dedicated work-queue. In this work-queue context, we merge
>> the participating filters and create a new filter that is logically
>> equal (match + actions). The merge itself is not complicated as it
>> might seems—TC does all the heavy lifting, this is not a random list
>> of filters. At this point, we configure the hardware with one
>> filter, either we have a match and the packet is handled by the
>> hardware, or we don’t and the packet goes to software unmodified.
>>
>> Going along this path we must tackle two things: 1) counters and 2)
>> TC filter deletion. 1) We must maintain TC counters as the user
>> expect. Each merged filter holds a list of filters it is derived
>> from, parents. Once an update is available for a merged filter
>> counter, the driver must update the corresponding parents
>> appropriately. 2) Upon TC filer deletion it is mandatory to remove
>> all the derived (merged) filters from the hardware as consequence.
> I'm failing to see how this would address two features of CT:
>
> - window validation. How would the card know that it should perform
> window validation on these packets?
The driver should allocate a context and mapped it appropriately, similar
way one should do for the MT approach. It is possible that one context
is shared between two or more merged flows. Context holds TCP state
in hardware.
> - flow stats. If it offloads the result of a merge, how can it
> retrieve the specific stats for each 5-tuple? Even with the lists
> you mentioned, once the 5-tuples are aggregated, we can't separate
> them anymore.
Each merged flow maintains a list of flows it is derived from, including
any 5-tuple involved. We still have an entry for each 5-tuple in the driver,
and we keep it up-to-date based on statistics from related merged flows.
Upper layers can query the driver for the relevant information given a
5-tuple.
Am I missing something?
>>
>>
>> Pros & Cons
>>
>> Pros: 1) Circumvent the complexity involved with continuation in
>> software where the hardware left off. 2) Simplifies the hardware
>> pipeline with only one filter and might improve the overall
>> performance.
>>
>> Cons: 1) Only applicable to OVS-oriented filters, will not support
>> priorities and overlapping filters. 2) Merger logic might consume
>> CPU cycles which might impact the rate of filters we can offload.
>> However, this overhead is believed to be negligible, if implemented
>> carefully. 3) Requires TC/flower to notify the driver on each filter
>> match (that is the only change needed above the driver).
> On more general comments, I have the feeling that this would be best
> done in OvS:
> - it has 2 flow "translations": one from OpenFlow to OvS/tc sw
> datapath, and then OvS/tc sw to hw. If the resulting flows are
> better for the hw, aren't them better for sw datapath too?
A valid point. I'm not sure by how much it will improve the slow-path
performance, but it will surely complicate the code. Doing this "trick"
definitely improves the hardware performance.
> - if this is for OvS-oriented filters, why can't/shouldn't OvS do this
> processing instead?
>
It is something we have considered before, as it makes sense. However,
going this way means for every new connection (different 5-tuple) a costly
"travel" to OVS user-space.
> Somehow I keep thinking this is replacing conntrack (as in the
> conntrack subsystem) with ipset offloading.
>
> All in all, any approach is probably fine but I'm struggling to see a
> path forward from this to actual CT offloading. Maybe I missed
> something.
> Cheers,
> Marcelo
>
>>
>>
>> Both approaches share the same software model, most of the code
>> above the driver is shared. This approach can be considered
>> temporary until the hardware will mature.
>>
>> What do you think about this approach?
>>
>> If something is not clear please let me know and I will do my best
>> to clarify.
>>
>> Cheers, Kuperman
>>
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] net/mlx5e: Remove 'parse_attr' argument in parse_tc_fdb_actions()
From: Roi Dayan @ 2019-02-26 13:38 UTC (permalink / raw)
To: xiangxia.m.yue@gmail.com, Saeed Mahameed, gerlitz.or@gmail.com
Cc: netdev@vger.kernel.org
In-Reply-To: <1551091207-10366-4-git-send-email-xiangxia.m.yue@gmail.com>
On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch is a little improvement. Simplify the parse_tc_fdb_actions().
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> index 708f819..e6583b9 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> @@ -2475,13 +2475,13 @@ static int parse_tc_vlan_action(struct mlx5e_priv *priv,
>
> static int parse_tc_fdb_actions(struct mlx5e_priv *priv,
> struct flow_action *flow_action,
> - struct mlx5e_tc_flow_parse_attr *parse_attr,
> struct mlx5e_tc_flow *flow,
> struct netlink_ext_ack *extack)
> {
> struct pedit_headers_action hdrs[2] = {};
> struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
> struct mlx5_esw_flow_attr *attr = flow->esw_attr;
> + struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr;
> struct mlx5e_rep_priv *rpriv = priv->ppriv;
> const struct ip_tunnel_info *info = NULL;
> const struct flow_action_entry *act;
> @@ -2796,7 +2796,7 @@ static bool is_peer_flow_needed(struct mlx5e_tc_flow *flow)
> if (err)
> goto err_free;
>
> - err = parse_tc_fdb_actions(priv, &rule->action, parse_attr, flow, extack);
> + err = parse_tc_fdb_actions(priv, &rule->action, flow, extack);
> if (err)
> goto err_free;
>
>
Reviewed-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* [PATCH net-next v4 2/4] arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the LS1028A RDB board
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
The LS1028A RDB board features an Atheros PHY connected over
SGMII to the ENETC PF0 (or Port0). ENETC Port1 (PF1) has no
external connection on this board, so it can be disabled for now.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2 - added a mdio node as parent for the phy node
v3 - none
v4 - none
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
index fdeb417..f86b054 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
@@ -71,3 +71,20 @@
&duart1 {
status = "okay";
};
+
+&enetc_port0 {
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+};
+
+&enetc_port1 {
+ status = "disabled";
+};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 4/4] dt-bindings: net: freescale: enetc: Add connection bindings for ENETC ethernet nodes
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
Define connection bindings (external PHY connections and internal links)
for the ENETC on-chip ethernet controllers.
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v3 - added this patch to the set
v4 - documented strictly the enetc node bindings, changed node type
from "pci" to "ethernet" and added compatible string (Rob H.)
.../devicetree/bindings/net/fsl-enetc.txt | 69 ++++++++++++++++++++++
1 file changed, 69 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/fsl-enetc.txt
diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt b/Documentation/devicetree/bindings/net/fsl-enetc.txt
new file mode 100644
index 0000000..c812e25
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
@@ -0,0 +1,69 @@
+* ENETC ethernet device tree bindings
+
+Depending on board design and ENETC port type (internal or
+external) there are two supported link modes specified by
+below device tree bindings.
+
+Required properties:
+
+- reg : Specifies PCIe Device Number and Function
+ Number of the ENETC endpoint device, according
+ to parent node bindings.
+- compatible : Should be "fsl,enetc".
+
+1) The ENETC external port is connected to a MDIO configurable phy:
+
+In this case, the ENETC node should include a "mdio" sub-node
+that in turn should contain the "ethernet-phy" node describing the
+external phy. Below properties are required, their bindings
+already defined in ethernet.txt or phy.txt, under
+Documentation/devicetree/bindings/net/*.
+
+Required:
+
+- phy-handle : Phandle to a PHY on the MDIO bus.
+ Defined in ethernet.txt.
+
+- phy-connection-type : Defined in ethernet.txt.
+
+- mdio : "mdio" node, defined in mdio.txt.
+
+- ethernet-phy : "ethernet-phy" node, defined in phy.txt.
+
+Example:
+
+ ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ phy-handle = <&sgmii_phy0>;
+ phy-connection-type = "sgmii";
+
+ mdio {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ sgmii_phy0: ethernet-phy@2 {
+ reg = <0x2>;
+ };
+ };
+ };
+
+2) The ENETC port is an internal port or has a fixed-link external
+connection:
+
+In this case, the ENETC port node defines a fixed link connection,
+as specified by "fixed-link.txt", under
+Documentation/devicetree/bindings/net/*.
+
+Required:
+
+- fixed-link : "fixed-link" node, defined in "fixed-link.txt".
+
+Example:
+ ethernet@0,2 {
+ compatible = "fsl,enetc";
+ reg = <0x000200 0 0 0 0>;
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 3/4] enetc: Add ENETC PF level external MDIO support
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
Each ENETC PF has its own MDIO interface, the corresponding
MDIO registers are mapped in the ENETC's Port register block.
The current patch adds a driver for these PF level MDIO buses,
so that each PF can manage directly its own external link.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
v2 - used readx_poll_timeout()
- added mdio node child to the port node
- added code comments, removed redundant err messages,
minor code cleanup
v3 - replaced dev_err w/ dev_dbg for read() issues
v4 - none
drivers/net/ethernet/freescale/enetc/Makefile | 3 +-
drivers/net/ethernet/freescale/enetc/enetc_mdio.c | 199 ++++++++++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 12 ++
drivers/net/ethernet/freescale/enetc/enetc_pf.h | 6 +
4 files changed, 219 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_mdio.c
diff --git a/drivers/net/ethernet/freescale/enetc/Makefile b/drivers/net/ethernet/freescale/enetc/Makefile
index 6976602..7139e41 100644
--- a/drivers/net/ethernet/freescale/enetc/Makefile
+++ b/drivers/net/ethernet/freescale/enetc/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_FSL_ENETC) += fsl-enetc.o
-fsl-enetc-$(CONFIG_FSL_ENETC) += enetc.o enetc_cbdr.o enetc_ethtool.o
+fsl-enetc-$(CONFIG_FSL_ENETC) += enetc.o enetc_cbdr.o enetc_ethtool.o \
+ enetc_mdio.o
fsl-enetc-$(CONFIG_PCI_IOV) += enetc_msg.o
fsl-enetc-objs := enetc_pf.o $(fsl-enetc-y)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_mdio.c b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
new file mode 100644
index 0000000..77b9cd1
--- /dev/null
+++ b/drivers/net/ethernet/freescale/enetc/enetc_mdio.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR BSD-3-Clause)
+/* Copyright 2019 NXP */
+
+#include <linux/mdio.h>
+#include <linux/of_mdio.h>
+#include <linux/iopoll.h>
+#include <linux/of.h>
+
+#include "enetc_pf.h"
+
+struct enetc_mdio_regs {
+ u32 mdio_cfg; /* MDIO configuration and status */
+ u32 mdio_ctl; /* MDIO control */
+ u32 mdio_data; /* MDIO data */
+ u32 mdio_addr; /* MDIO address */
+};
+
+#define bus_to_enetc_regs(bus) (struct enetc_mdio_regs __iomem *)((bus)->priv)
+
+#define ENETC_MDIO_REG_OFFSET 0x1c00
+#define ENETC_MDC_DIV 258
+
+#define MDIO_CFG_CLKDIV(x) ((((x) >> 1) & 0xff) << 8)
+#define MDIO_CFG_BSY BIT(0)
+#define MDIO_CFG_RD_ER BIT(1)
+#define MDIO_CFG_ENC45 BIT(6)
+ /* external MDIO only - driven on neg MDC edge */
+#define MDIO_CFG_NEG BIT(23)
+
+#define MDIO_CTL_DEV_ADDR(x) ((x) & 0x1f)
+#define MDIO_CTL_PORT_ADDR(x) (((x) & 0x1f) << 5)
+#define MDIO_CTL_READ BIT(15)
+#define MDIO_DATA(x) ((x) & 0xffff)
+
+#define TIMEOUT 1000
+static int enetc_mdio_wait_complete(struct enetc_mdio_regs __iomem *regs)
+{
+ u32 val;
+
+ return readx_poll_timeout(enetc_rd_reg, ®s->mdio_cfg, val,
+ !(val & MDIO_CFG_BSY), 10, 10 * TIMEOUT);
+}
+
+static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
+ u16 value)
+{
+ struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ u32 mdio_ctl, mdio_cfg;
+ u16 dev_addr;
+ int ret;
+
+ mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
+ if (regnum & MII_ADDR_C45) {
+ dev_addr = (regnum >> 16) & 0x1f;
+ mdio_cfg |= MDIO_CFG_ENC45;
+ } else {
+ /* clause 22 (ie 1G) */
+ dev_addr = regnum & 0x1f;
+ mdio_cfg &= ~MDIO_CFG_ENC45;
+ }
+
+ enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* set port and dev addr */
+ mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+
+ /* set the register address */
+ if (regnum & MII_ADDR_C45) {
+ enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+ }
+
+ /* write the value */
+ enetc_wr_reg(®s->mdio_data, MDIO_DATA(value));
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int enetc_mdio_read(struct mii_bus *bus, int phy_id, int regnum)
+{
+ struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
+ u32 mdio_ctl, mdio_cfg;
+ u16 dev_addr, value;
+ int ret;
+
+ mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
+ if (regnum & MII_ADDR_C45) {
+ dev_addr = (regnum >> 16) & 0x1f;
+ mdio_cfg |= MDIO_CFG_ENC45;
+ } else {
+ dev_addr = regnum & 0x1f;
+ mdio_cfg &= ~MDIO_CFG_ENC45;
+ }
+
+ enetc_wr_reg(®s->mdio_cfg, mdio_cfg);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* set port and device addr */
+ mdio_ctl = MDIO_CTL_PORT_ADDR(phy_id) | MDIO_CTL_DEV_ADDR(dev_addr);
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl);
+
+ /* set the register address */
+ if (regnum & MII_ADDR_C45) {
+ enetc_wr_reg(®s->mdio_addr, regnum & 0xffff);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+ }
+
+ /* initiate the read */
+ enetc_wr_reg(®s->mdio_ctl, mdio_ctl | MDIO_CTL_READ);
+
+ ret = enetc_mdio_wait_complete(regs);
+ if (ret)
+ return ret;
+
+ /* return all Fs if nothing was there */
+ if (enetc_rd_reg(®s->mdio_cfg) & MDIO_CFG_RD_ER) {
+ dev_dbg(&bus->dev,
+ "Error while reading PHY%d reg at %d.%hhu\n",
+ phy_id, dev_addr, regnum);
+ return 0xffff;
+ }
+
+ value = enetc_rd_reg(®s->mdio_data) & 0xffff;
+
+ return value;
+}
+
+int enetc_mdio_probe(struct enetc_pf *pf)
+{
+ struct device *dev = &pf->si->pdev->dev;
+ struct enetc_mdio_regs __iomem *regs;
+ struct device_node *np;
+ struct mii_bus *bus;
+ int ret;
+
+ bus = mdiobus_alloc_size(sizeof(regs));
+ if (!bus)
+ return -ENOMEM;
+
+ bus->name = "Freescale ENETC MDIO Bus";
+ bus->read = enetc_mdio_read;
+ bus->write = enetc_mdio_write;
+ bus->parent = dev;
+ snprintf(bus->id, MII_BUS_ID_SIZE, "%s", dev_name(dev));
+
+ /* store the enetc mdio base address for this bus */
+ regs = pf->si->hw.port + ENETC_MDIO_REG_OFFSET;
+ bus->priv = regs;
+
+ np = of_get_child_by_name(dev->of_node, "mdio");
+ if (!np) {
+ dev_err(dev, "MDIO node missing\n");
+ ret = -EINVAL;
+ goto err_registration;
+ }
+
+ ret = of_mdiobus_register(bus, np);
+ if (ret) {
+ of_node_put(np);
+ dev_err(dev, "cannot register MDIO bus\n");
+ goto err_registration;
+ }
+
+ of_node_put(np);
+ pf->mdio = bus;
+
+ return 0;
+
+err_registration:
+ mdiobus_free(bus);
+
+ return ret;
+}
+
+void enetc_mdio_remove(struct enetc_pf *pf)
+{
+ if (pf->mdio) {
+ mdiobus_unregister(pf->mdio);
+ mdiobus_free(pf->mdio);
+ }
+}
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 7d28f5e..15876a6 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -746,6 +746,7 @@ static void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
{
+ struct enetc_pf *pf = enetc_si_priv(priv->si);
struct device_node *np = priv->dev->of_node;
int err;
@@ -770,12 +771,22 @@ static int enetc_of_get_phy(struct enetc_ndev_priv *priv)
priv->phy_node = of_node_get(np);
}
+ if (!of_phy_is_fixed_link(np)) {
+ err = enetc_mdio_probe(pf);
+ if (err) {
+ of_node_put(priv->phy_node);
+ return err;
+ }
+ }
+
priv->if_mode = of_get_phy_mode(np);
if (priv->if_mode < 0) {
dev_err(priv->dev, "missing phy type\n");
of_node_put(priv->phy_node);
if (of_phy_is_fixed_link(np))
of_phy_deregister_fixed_link(np);
+ else
+ enetc_mdio_remove(pf);
return -EINVAL;
}
@@ -898,6 +909,7 @@ static void enetc_pf_remove(struct pci_dev *pdev)
unregister_netdev(si->ndev);
+ enetc_mdio_remove(pf);
enetc_of_put_phy(priv);
enetc_free_msix(priv);
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.h b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
index 2061ae5..10dd1b5 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.h
@@ -42,8 +42,14 @@ struct enetc_pf {
char vlan_promisc_simap; /* bitmap of SIs in VLAN promisc mode */
DECLARE_BITMAP(vlan_ht_filter, ENETC_VLAN_HT_SIZE);
DECLARE_BITMAP(active_vlans, VLAN_N_VID);
+
+ struct mii_bus *mdio; /* saved for cleanup */
};
int enetc_msg_psi_init(struct enetc_pf *pf);
void enetc_msg_psi_free(struct enetc_pf *pf);
void enetc_msg_handle_rxmsg(struct enetc_pf *pf, int mbox_id, u16 *status);
+
+/* MDIO */
+int enetc_mdio_probe(struct enetc_pf *pf);
+void enetc_mdio_remove(struct enetc_pf *pf);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 1/4] arm64: dts: fsl: ls1028a: Add PCI IERC node and ENETC endpoints
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
In-Reply-To: <1551188543-28867-1-git-send-email-claudiu.manoil@nxp.com>
The LS1028A SoC features a PCI Integrated Endpoint Root Complex
(IERC) defining several integrated PCI devices, including the ENETC
ethernet controller integrated endpoints (IEPs). The IERC implements
ECAM (Enhanced Configuration Access Mechanism) to provide access
to the PCIe config space of the IEPs. This means the the IEPs
(including ENETC) do not support the standard PCIe BARs, instead
the Enhanced Allocation (EA) capability structures in the ECAM space
are used to fix the base addresses in the system, and the PCI
subsystem uses these structures for device enumeration and discovery.
The "ranges" entries contain basic information from these EA capabily
structures required by the kernel for device enumeration.
The current patch also enables the first 2 ENETC PFs (Physiscal
Functions) and the associated VFs (Virtual Functions), 2 VFs for
each PF. Each of these ENETC PFs has an external ethernet port
on the LS1028A SoC.
Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2 - none
v3 - none
v4 - changed enetc node names from "pci" to "ethernet" and
added compatible string (as requested by Rob H.)
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 35 ++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
index a8cf92a..2896bbc 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi
@@ -335,5 +335,40 @@
<GIC_SPI 206 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 207 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 208 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 209 IRQ_TYPE_LEVEL_HIGH>;
};
+
+ pcie@1f0000000 { /* Integrated Endpoint Root Complex */
+ compatible = "pci-host-ecam-generic";
+ reg = <0x01 0xf0000000 0x0 0x100000>;
+ #address-cells = <3>;
+ #size-cells = <2>;
+ #interrupt-cells = <1>;
+ msi-parent = <&its>;
+ device_type = "pci";
+ bus-range = <0x0 0x0>;
+ dma-coherent;
+ msi-map = <0 &its 0x17 0xe>;
+ iommu-map = <0 &smmu 0x17 0xe>;
+ /* PF0-6 BAR0 - non-prefetchable memory */
+ ranges = <0x82000000 0x0 0x00000000 0x1 0xf8000000 0x0 0x160000
+ /* PF0-6 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf8160000 0x0 0x070000
+ /* PF0: VF0-1 BAR0 - non-prefetchable memory */
+ 0x82000000 0x0 0x00000000 0x1 0xf81d0000 0x0 0x020000
+ /* PF0: VF0-1 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf81f0000 0x0 0x020000
+ /* PF1: VF0-1 BAR0 - non-prefetchable memory */
+ 0x82000000 0x0 0x00000000 0x1 0xf8210000 0x0 0x020000
+ /* PF1: VF0-1 BAR2 - prefetchable memory */
+ 0xc2000000 0x0 0x00000000 0x1 0xf8230000 0x0 0x020000>;
+
+ enetc_port0: ethernet@0,0 {
+ compatible = "fsl,enetc";
+ reg = <0x000000 0 0 0 0>;
+ };
+ enetc_port1: ethernet@0,1 {
+ compatible = "fsl,enetc";
+ reg = <0x000100 0 0 0 0>;
+ };
+ };
};
};
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v4 0/4] enetc: Add mdio support and device tree nodes
From: Claudiu Manoil @ 2019-02-26 13:42 UTC (permalink / raw)
To: Rob Herring, Shawn Guo, Li Yang, David S . Miller
Cc: alexandru.marginean, Andrew Lunn, linux-arm-kernel, devicetree,
netdev, linux-kernel
This is the missing part to enable PCI probing of the ENETC ethernet
ports on the LS1028A SoC and external traffic on the LS1028A RDB board.
It's one of the first items on the TODO list for the recently merged
ENETC ethernet driver.
v3: Add DT bindings doc for ENETC connections
v4: none
Claudiu Manoil (4):
arm64: dts: fsl: ls1028a: Add PCI IERC node and ENETC endpoints
arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the
LS1028A RDB board
enetc: Add ENETC PF level external MDIO support
dt-bindings: net: freescale: enetc: Add connection bindings for ENETC
ethernet nodes
.../devicetree/bindings/net/fsl-enetc.txt | 69 +++++++
arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 17 ++
arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi | 35 ++++
drivers/net/ethernet/freescale/enetc/Makefile | 3 +-
drivers/net/ethernet/freescale/enetc/enetc_mdio.c | 199 +++++++++++++++++++++
drivers/net/ethernet/freescale/enetc/enetc_pf.c | 12 ++
drivers/net/ethernet/freescale/enetc/enetc_pf.h | 6 +
7 files changed, 340 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/net/fsl-enetc.txt
create mode 100644 drivers/net/ethernet/freescale/enetc/enetc_mdio.c
--
2.7.4
^ permalink raw reply
* Re: [PATCH net 3/4] tls: Fix mixing between async capable and async
From: Boris Pismenny @ 2019-02-26 13:43 UTC (permalink / raw)
To: Vakul Garg, Aviad Yehezkel, davejwatson@fb.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
netdev@vger.kernel.org
Cc: Eran Ben Elisha
In-Reply-To: <DB7PR04MB4252FD5C491CA99AF45BD10C8B7B0@DB7PR04MB4252.eurprd04.prod.outlook.com>
On 2/26/2019 2:38 PM, Vakul Garg wrote:
>
>
>> -----Original Message-----
>> From: Boris Pismenny <borisp@mellanox.com>
>> Sent: Tuesday, February 26, 2019 5:43 PM
>> To: aviadye@mellanox.com; davejwatson@fb.com;
>> john.fastabend@gmail.com; daniel@iogearbox.net; Vakul Garg
>> <vakul.garg@nxp.com>; netdev@vger.kernel.org
>> Cc: eranbe@mellanox.com; borisp@mellanox.com
>> Subject: [PATCH net 3/4] tls: Fix mixing between async capable and async
>>
>> From: Eran Ben Elisha <eranbe@mellanox.com>
>>
>> Today, tls_sw_recvmsg is capable of using asynchronous mode to handle
>> application data TLS records. Moreover, it assumes that if the cipher can be
>> handled asynchronously, then all packets will be processed asynchronously.
>>
>> However, this assumption is not always true.
>
> Could you please elaborate, what happens?
>
When decryption doesn't occur asynchronously e.g. return code is not
EINPROGRESS, then async should be turned off.
>> Specifically, for AES-GCM in
>> TLS1.2, it causes data corruption, and breaks user applications.
>>
>> This patch fixes this problem by separating the async capability from the
>> decryption operation result.
>>
>> Fixes: c0ab4732d4c6 ("net/tls: Do not use async crypto for non-data
>> records")
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> Reviewed-by: Boris Pismenny <borisp@mellanox.com>
>> ---
>> net/tls/tls_sw.c | 15 +++++++++------
>> 1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c index
>> 4afa67b00aaf..f515cd7e984e 100644
>> --- a/net/tls/tls_sw.c
>> +++ b/net/tls/tls_sw.c
>> @@ -1693,7 +1693,8 @@ int tls_sw_recvmsg(struct sock *sk,
>> bool zc = false;
>> int to_decrypt;
>> int chunk = 0;
>> - bool async;
>> + bool async_capable;
>> + bool async = false;
>>
>> skb = tls_wait_data(sk, psock, flags, timeo, &err);
>> if (!skb) {
>> @@ -1727,21 +1728,23 @@ int tls_sw_recvmsg(struct sock *sk,
>>
>> /* Do not use async mode if record is non-data */
>> if (ctx->control == TLS_RECORD_TYPE_DATA)
>> - async = ctx->async_capable;
>> + async_capable = ctx->async_capable;
>> else
>> - async = false;
>> + async_capable = false;
>>
>> err = decrypt_skb_update(sk, skb, &msg->msg_iter,
>> - &chunk, &zc, async);
>> + &chunk, &zc, async_capable);
>> if (err < 0 && err != -EINPROGRESS) {
>> tls_err_abort(sk, EBADMSG);
>> goto recv_end;
>> }
>>
>> - if (err == -EINPROGRESS)
>> + if (err == -EINPROGRESS && async_capable) {
> Why do we need to check 'async_capable'?
> Do we get err == -EINPROGRESS even when async_capable is false?
>
I've missed this. I'll remove this and send V2.
>> + async = true;
>> num_async++;
>> - else if (prot->version == TLS_1_3_VERSION)
>> + } else if (prot->version == TLS_1_3_VERSION) {
>> tlm->control = ctx->control;
>> + }
>>
>> /* If the type of records being processed is not known yet,
>> * set it to record type just dequeued. If it is already known,
>> --
>> 2.12.2
>
>
>
>
^ 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