* [PATCH net-next] ipv6: Allow the l3mdev to be a loopback
From: Mike Manning @ 2018-09-19 12:56 UTC (permalink / raw)
To: netdev; +Cc: Robert Shearman
From: Robert Shearman <rshearma@vyatta.att-mail.com>
There is no way currently for an IPv6 client connect using a loopback
address in a VRF, whereas for IPv4 the loopback address can be added:
$ sudo ip addr add dev vrfred 127.0.0.1/8
$ sudo ip -6 addr add ::1/128 dev vrfred
RTNETLINK answers: Cannot assign requested address
So allow ::1 to be configured on an L3 master device. In order for
this to be usable ip_route_output_flags needs to not consider ::1 to
be a link scope address (since oif == l3mdev and so it would be
dropped), and ipv6_rcv needs to consider the l3mdev to be a loopback
device so that it doesn't drop the packets.
Signed-off-by: Robert Shearman <rshearma@vyatta.att-mail.com>
Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
---
net/ipv6/addrconf.c | 1 +
net/ipv6/ip6_input.c | 3 ++-
net/ipv6/route.c | 3 ++-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d4733160e6b7..bfe3ec7ecb14 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -997,6 +997,7 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
if (addr_type == IPV6_ADDR_ANY ||
addr_type & IPV6_ADDR_MULTICAST ||
(!(idev->dev->flags & IFF_LOOPBACK) &&
+ !netif_is_l3_master(idev->dev) &&
addr_type & IPV6_ADDR_LOOPBACK))
return ERR_PTR(-EADDRNOTAVAIL);
diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index 6242682be876..96577e742afd 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -178,7 +178,8 @@ static struct sk_buff *ip6_rcv_core(struct sk_buff *skb, struct net_device *dev,
*/
if ((ipv6_addr_loopback(&hdr->saddr) ||
ipv6_addr_loopback(&hdr->daddr)) &&
- !(dev->flags & IFF_LOOPBACK))
+ !(dev->flags & IFF_LOOPBACK) &&
+ !netif_is_l3_master(dev))
goto err;
/* RFC4291 Errata ID: 3480
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 0fa62acc923c..f36ee8a3314f 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2098,7 +2098,8 @@ struct dst_entry *ip6_route_output_flags(struct net *net, const struct sock *sk,
{
bool any_src;
- if (rt6_need_strict(&fl6->daddr)) {
+ if (ipv6_addr_type(&fl6->daddr) &
+ (IPV6_ADDR_MULTICAST | IPV6_ADDR_LINKLOCAL)) {
struct dst_entry *dst;
dst = l3mdev_link_scope_lookup(net, fl6);
--
2.11.0
^ permalink raw reply related
* Re: Is it possible to get Rx timestamps in skb->tstamp?
From: David Howells @ 2018-09-19 13:43 UTC (permalink / raw)
To: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?=; +Cc: dhowells, netdev
In-Reply-To: <875zz1k4pv.fsf@toke.dk>
Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> > Is it possible to tell a UDP socket that you'd like it to put
> > reception timestamps in skb->tstamp?
>
> I think you probably want SO_TIMESTAMP*?
>
> See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/timestamping.txt
That seems to work. I thought that only affected recvmsg() putting the
timestamp into the ancillary data buffer, but apparently not.
Thanks,
David
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Timur Tabi @ 2018-09-19 14:05 UTC (permalink / raw)
To: Andrew Lunn, Wang, Dongsheng
Cc: Florian Fainelli, davem@davemloft.net, Zheng, Joey,
netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20180919122521.GA26940@lunn.ch>
On 9/19/18 7:25 AM, Andrew Lunn wrote:
> ACPI is completely separate and should not affect the DT binding.
> I've not yet looked at the ACPI changes you added.
Just FYI, there is no device tree platform on which the upstream EMAC
driver works. All of the DT code in the driver is theoretical. It
worked once on a prototype platform, when I originally wrote the code,
but since then DT support is mostly a guess.
The focus of any patches for the EMAC should be ACPI, not DT. If
anything, ACPI support should come first. No one should be writing or
reviewing DT code before ACPI code.
The upstream EMAC driver is only known to work on the QDF2400, which is
an ACPI-only chip. I feel like I've been repeating this too often.
^ permalink raw reply
* [PATCH v2 1/5] netlink: remove NLA_NESTED_COMPAT
From: Johannes Berg @ 2018-09-19 19:49 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David Ahern, Johannes Berg
In-Reply-To: <20180919194905.16462-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
This isn't used anywhere, so we might as well get rid of it.
Reviewed-by: David Ahern <dsahern-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
include/net/netlink.h | 2 --
lib/nlattr.c | 11 -----------
2 files changed, 13 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 318b1ded3833..b680fe365e91 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -172,7 +172,6 @@ enum {
NLA_FLAG,
NLA_MSECS,
NLA_NESTED,
- NLA_NESTED_COMPAT,
NLA_NUL_STRING,
NLA_BINARY,
NLA_S8,
@@ -203,7 +202,6 @@ enum {
* NLA_BINARY Maximum length of attribute payload
* NLA_NESTED Don't use `len' field -- length verification is
* done by checking len of nested header (or empty)
- * NLA_NESTED_COMPAT Minimum length of structure payload
* NLA_U8, NLA_U16,
* NLA_U32, NLA_U64,
* NLA_S8, NLA_S16,
diff --git a/lib/nlattr.c b/lib/nlattr.c
index bb6fe5ed4ecf..120ad569e13d 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -140,17 +140,6 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
return -ERANGE;
break;
- case NLA_NESTED_COMPAT:
- if (attrlen < pt->len)
- return -ERANGE;
- if (attrlen < NLA_ALIGN(pt->len))
- break;
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN)
- return -ERANGE;
- nla = nla_data(nla) + NLA_ALIGN(pt->len);
- if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN + nla_len(nla))
- return -ERANGE;
- break;
case NLA_NESTED:
/* a nested attributes is allowed to be empty; if its not,
* it must have a size of at least NLA_HDRLEN.
--
2.14.4
^ permalink raw reply related
* [PATCH v2 2/5] netlink: make validation_data const
From: Johannes Berg @ 2018-09-19 19:49 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David Ahern, Johannes Berg
In-Reply-To: <20180919194905.16462-1-johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
The validation data is only used within the policy that
should usually already be const, and isn't changed in any
code that uses it. Therefore, make the validation_data
pointer const.
While at it, remove the duplicate variable in the bitfield
validation that I'd otherwise have to change to const.
Reviewed-by: David Ahern <dsahern-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Signed-off-by: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
include/net/netlink.h | 2 +-
lib/nlattr.c | 5 ++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index b680fe365e91..0d698215d4d9 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -237,7 +237,7 @@ enum {
struct nla_policy {
u16 type;
u16 len;
- void *validation_data;
+ const void *validation_data;
};
#define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_EXACT_LEN, .len = _len }
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 120ad569e13d..e2e5b38394d5 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -45,12 +45,11 @@ static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
};
static int validate_nla_bitfield32(const struct nlattr *nla,
- u32 *valid_flags_allowed)
+ const u32 *valid_flags_mask)
{
const struct nla_bitfield32 *bf = nla_data(nla);
- u32 *valid_flags_mask = valid_flags_allowed;
- if (!valid_flags_allowed)
+ if (!valid_flags_mask)
return -EINVAL;
/*disallow invalid bit selector */
--
2.14.4
^ permalink raw reply related
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Michal Kubecek @ 2018-09-19 14:41 UTC (permalink / raw)
To: Edward Cree
Cc: John W. Linville, netdev, Ganesh Goudar, Jakub Kicinski,
Dustin Byford, Dirk van der Merwe
In-Reply-To: <518b8b8b-0151-1053-3798-6009044ed53a@solarflare.com>
On Wed, Sep 05, 2018 at 06:54:57PM +0100, Edward Cree wrote:
> Of the three drivers that currently support FEC configuration, two (sfc
> and cxgb4[vf]) accept configurations with more than one bit set in the
> feccmd.fec bitmask. (The precise semantics of these combinations vary.)
> Thus, this patch adds the ability to specify such combinations through a
> comma-separated list of FEC modes in the 'encoding' argument on the
> command line.
>
> Also adds --set-fec tests to test-cmdline.c, and corrects the man page
> (the encoding argument is not optional) while updating it.
>
> Signed-off-by: Edward Cree <ecree@solarflare.com>
...
> +/* Takes a comma-separated list of FEC modes, returns the bitwise OR of their
> + * corresponding ETHTOOL_FEC_* constants.
> + * Accepts repetitions (e.g. 'auto,auto') and trailing comma (e.g. 'off,').
> + */
> +static int parse_fecmode(const char *str)
> +{
> int fecmode = 0;
> + char buf[6];
>
> if (!str)
> - return fecmode;
> -
> - if (!strcasecmp(str, "auto"))
> - fecmode |= ETHTOOL_FEC_AUTO;
> - else if (!strcasecmp(str, "off"))
> - fecmode |= ETHTOOL_FEC_OFF;
> - else if (!strcasecmp(str, "rs"))
> - fecmode |= ETHTOOL_FEC_RS;
> - else if (!strcasecmp(str, "baser"))
> - fecmode |= ETHTOOL_FEC_BASER;
> + return 0;
> + while (*str) {
> + size_t next;
> + int mode;
>
> + next = strcspn(str, ",");
> + if (next >= 6) /* Bad mode, longest name is 5 chars */
> + return 0;
> + /* Copy into temp buffer and nul-terminate */
> + memcpy(buf, str, next);
> + buf[next] = 0;
> + mode = fecmode_str_to_type(buf);
> + if (!mode) /* Bad mode encountered */
> + return 0;
> + fecmode |= mode;
> + str += next;
> + /* Skip over ',' (but not nul) */
> + if (*str)
> + str++;
> + }
> return fecmode;
> }
>
I'm sorry I didn't notice this before the patch was accepted but as it's
not in a release yet, maybe it's still not too late.
Could I suggest to make the syntax consistent with other options? I mean
rather than a comma separated list to use either
ethtool --set-fec <dev> encoding enc1 enc2 ...
(as we have for --reset) or
ethtool --set-fec <dev> encoding enc1 on|off enc2 on|off ...
(as we have e.g. for msglvl, -K or --set-eee)?
Second option seems more appropriate to me but it would require special
handling of the case when there is only one argument after "encoding" to
maintain backward compatibility with already released versions.
One loosely related question: how sure can we be that we are never going
to need more than 32 bits for FEC encodings? Is it something completely
hypothetical or is it something that could happen in the future?
Michal Kubecek
^ permalink raw reply
* Re: [RFC bpf-next 2/4] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Alexei Starovoitov @ 2018-09-19 15:14 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
Quentin Monnet, David S . Miller, netdev
In-Reply-To: <20180919075143.9308-3-bhole_prashant_q7@lab.ntt.co.jp>
On Wed, Sep 19, 2018 at 04:51:41PM +0900, Prashant Bhole wrote:
> Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
> map types:
> - BPF_MAP_TYPE_PROG_ARRAY
> - BPF_MAP_TYPE_STACK_TRACE
> - BPF_MAP_TYPE_XSKMAP
> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> kernel/bpf/arraymap.c | 2 +-
> kernel/bpf/sockmap.c | 2 +-
> kernel/bpf/stackmap.c | 2 +-
> kernel/bpf/xskmap.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index dded84cbe814..24583da9ffd1 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
>
> static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
> {
> - return NULL;
> + return ERR_PTR(-EOPNOTSUPP);
> }
conceptually the set looks good to me.
Please add a test to test_verifier.c to make sure
that these lookup helpers cannot be called from BPF program.
Otherwise this diff may cause crashes.
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Andrew Lunn @ 2018-09-19 15:20 UTC (permalink / raw)
To: Timur Tabi
Cc: Wang, Dongsheng, Florian Fainelli, davem@davemloft.net,
Zheng, Joey, netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <1226d3d3-e2ba-571d-5b70-b246cbde2fb6@kernel.org>
> The focus of any patches for the EMAC should be ACPI, not DT. If anything,
> ACPI support should come first. No one should be writing or reviewing DT
> code before ACPI code.
I suspect that is not going to be easy. Last time i looked, the ACPI
standard had nothing about MDIO busses or PHYs. Marcin Wojtas did some
work in this area a while back for the mvpp2, but if i remember
correctly, he worked around this by simply not having a PHY when using
ACPI, and making use of a MAC interrupt which indicated when there was
link.
Whoever implements this first needs to be an ACPI expert and probably
needs to write it up and submit it as an amendment to the ACPI
standard.
Andrew
^ permalink raw reply
* [PATCH net-next] net/tls: Add support for async encryption of records for performance
From: Vakul Garg @ 2018-09-19 15:21 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, doronrk, Vakul Garg
In current implementation, tls records are encrypted & transmitted
serially. Till the time the previously submitted user data is encrypted,
the implementation waits and on finish starts transmitting the record.
This approach of encrypt-one record at a time is inefficient when
asynchronous crypto accelerators are used. For each record, there are
overheads of interrupts, driver softIRQ scheduling etc. Also the crypto
accelerator sits idle most of time while an encrypted record's pages are
handed over to tcp stack for transmission.
This patch enables encryption of multiple records in parallel when an
async capable crypto accelerator is present in system. This is achieved
by allowing the user space application to send more data using sendmsg()
even while previously issued data is being processed by crypto
accelerator. This requires returning the control back to user space
application after submitting encryption request to accelerator. This
also means that zero-copy mode of encryption cannot be used with async
accelerator as we must be done with user space application buffer before
returning from sendmsg().
There can be multiple records in flight to/from the accelerator. Each of
the record is represented by 'struct tls_rec'. This is used to store the
memory pages for the record.
After the records are encrypted, they are added in a linked list called
tx_ready_list which contains encrypted tls records sorted as per tls
sequence number. The records from tx_ready_list are transmitted using a
newly introduced function called tls_tx_records(). The tx_ready_list is
polled for any record ready to be transmitted in sendmsg(), sendpage()
after initiating encryption of new tls records. This achieves parallel
encryption and transmission of records when async accelerator is
present.
There could be situation when crypto accelerator completes encryption
later than polling of tx_ready_list by sendmsg()/sendpage(). Therefore
we need a deferred work context to be able to transmit records from
tx_ready_list. The deferred work context gets scheduled if applications
are not sending much data through the socket. If the applications issue
sendmsg()/sendpage() in quick succession, then the scheduling of
tx_work_handler gets cancelled as the tx_ready_list would be polled from
application's context itself. This saves scheduling overhead of deferred
work.
The patch also brings some side benefit. We are able to get rid of the
concept of CLOSED record. This is because the records once closed are
either encrypted and then placed into tx_ready_list or if encryption
fails, the socket error is set. This simplifies the kernel tls
sendpath. However since tls_device.c is still using macros, accessory
functions for CLOSED records have been retained.
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
include/net/tls.h | 70 +++++--
net/tls/tls_main.c | 54 ++---
net/tls/tls_sw.c | 569 ++++++++++++++++++++++++++++++++++++++++-------------
3 files changed, 515 insertions(+), 178 deletions(-)
diff --git a/include/net/tls.h b/include/net/tls.h
index 9f3c4ea9ad6f..84756667fc2a 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -41,7 +41,7 @@
#include <linux/tcp.h>
#include <net/tcp.h>
#include <net/strparser.h>
-
+#include <crypto/aead.h>
#include <uapi/linux/tls.h>
@@ -93,24 +93,47 @@ enum {
TLS_NUM_CONFIG,
};
-struct tls_sw_context_tx {
- struct crypto_aead *aead_send;
- struct crypto_wait async_wait;
-
- char aad_space[TLS_AAD_SPACE_SIZE];
-
- unsigned int sg_plaintext_size;
- int sg_plaintext_num_elem;
+/* TLS records are maintained in 'struct tls_rec'. It stores the memory pages
+ * allocated or mapped for each TLS record. After encryption, the records are
+ * stores in a linked list.
+ */
+struct tls_rec {
+ struct list_head list;
+ int tx_flags;
struct scatterlist sg_plaintext_data[MAX_SKB_FRAGS];
-
- unsigned int sg_encrypted_size;
- int sg_encrypted_num_elem;
struct scatterlist sg_encrypted_data[MAX_SKB_FRAGS];
/* AAD | sg_plaintext_data | sg_tag */
struct scatterlist sg_aead_in[2];
/* AAD | sg_encrypted_data (data contain overhead for hdr&iv&tag) */
struct scatterlist sg_aead_out[2];
+
+ unsigned int sg_plaintext_size;
+ unsigned int sg_encrypted_size;
+ int sg_plaintext_num_elem;
+ int sg_encrypted_num_elem;
+
+ char aad_space[TLS_AAD_SPACE_SIZE];
+ struct aead_request aead_req;
+ u8 aead_req_ctx[];
+};
+
+struct tx_work {
+ struct delayed_work work;
+ struct sock *sk;
+};
+
+struct tls_sw_context_tx {
+ struct crypto_aead *aead_send;
+ struct crypto_wait async_wait;
+ struct tx_work tx_work;
+ struct tls_rec *open_rec;
+ struct list_head tx_ready_list;
+ atomic_t encrypt_pending;
+ int async_notify;
+
+#define BIT_TX_SCHEDULED 0
+ unsigned long tx_bitmask;
};
struct tls_sw_context_rx {
@@ -197,6 +220,8 @@ struct tls_context {
struct scatterlist *partially_sent_record;
u16 partially_sent_offset;
+ u64 tx_seq_number; /* Next TLS seqnum to be transmitted */
+
unsigned long flags;
bool in_tcp_sendpages;
@@ -261,6 +286,7 @@ int tls_device_sendpage(struct sock *sk, struct page *page,
void tls_device_sk_destruct(struct sock *sk);
void tls_device_init(void);
void tls_device_cleanup(void);
+int tls_tx_records(struct sock *sk, int flags);
struct tls_record_info *tls_get_record(struct tls_offload_context_tx *context,
u32 seq, u64 *p_record_sn);
@@ -279,6 +305,9 @@ 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);
+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);
@@ -312,6 +341,23 @@ static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
return tls_ctx->pending_open_record_frags;
}
+static inline bool is_tx_ready(struct tls_context *tls_ctx,
+ struct tls_sw_context_tx *ctx)
+{
+ struct tls_rec *rec;
+ u64 seq;
+
+ rec = list_first_entry(&ctx->tx_ready_list, struct tls_rec, list);
+ if (!rec)
+ return false;
+
+ seq = be64_to_cpup((const __be64 *)&rec->aad_space);
+ if (seq == tls_ctx->tx_seq_number)
+ return true;
+ else
+ return false;
+}
+
struct sk_buff *
tls_validate_xmit_skb(struct sock *sk, struct net_device *dev,
struct sk_buff *skb);
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 523622dc74f8..06094de7a3d9 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -141,7 +141,6 @@ int tls_push_sg(struct sock *sk,
size = sg->length;
}
- clear_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
ctx->in_tcp_sendpages = false;
ctx->sk_write_space(sk);
@@ -193,15 +192,12 @@ int tls_proccess_cmsg(struct sock *sk, struct msghdr *msg,
return rc;
}
-int tls_push_pending_closed_record(struct sock *sk, struct tls_context *ctx,
- int flags, long *timeo)
+int tls_push_partial_record(struct sock *sk, struct tls_context *ctx,
+ int flags)
{
struct scatterlist *sg;
u16 offset;
- if (!tls_is_partially_sent_record(ctx))
- return ctx->push_pending_record(sk, flags);
-
sg = ctx->partially_sent_record;
offset = ctx->partially_sent_offset;
@@ -209,9 +205,23 @@ int tls_push_pending_closed_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_ready_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);
+ struct tls_sw_context_tx *tx_ctx = tls_sw_ctx_tx(ctx);
/* If in_tcp_sendpages call lower protocol write space handler
* to ensure we wake up any waiting operations there. For example
@@ -222,20 +232,11 @@ static void tls_write_space(struct sock *sk)
return;
}
- if (!sk->sk_write_pending && tls_is_pending_closed_record(ctx)) {
- gfp_t sk_allocation = sk->sk_allocation;
- int rc;
- long timeo = 0;
-
- sk->sk_allocation = GFP_ATOMIC;
- rc = tls_push_pending_closed_record(sk, ctx,
- MSG_DONTWAIT |
- MSG_NOSIGNAL,
- &timeo);
- sk->sk_allocation = sk_allocation;
-
- if (rc < 0)
- return;
+ /* Schedule the transmission if tx list is ready */
+ if (is_tx_ready(ctx, 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);
}
ctx->sk_write_space(sk);
@@ -270,19 +271,6 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
if (!tls_complete_pending_work(sk, ctx, 0, &timeo))
tls_handle_open_record(sk, 0);
- if (ctx->partially_sent_record) {
- struct scatterlist *sg = ctx->partially_sent_record;
-
- while (1) {
- put_page(sg_page(sg));
- sk_mem_uncharge(sk, sg->length);
-
- if (sg_is_last(sg))
- break;
- sg++;
- }
- }
-
/* We need these for tls_sw_fallback handling of other packets */
if (ctx->tx_conf == TLS_SW) {
kfree(ctx->tx.rec_seq);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 5ff51bac8b46..8c79805daa63 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -246,18 +246,19 @@ static void trim_both_sgl(struct sock *sk, int target_size)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec = ctx->open_rec;
- trim_sg(sk, ctx->sg_plaintext_data,
- &ctx->sg_plaintext_num_elem,
- &ctx->sg_plaintext_size,
+ trim_sg(sk, rec->sg_plaintext_data,
+ &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size,
target_size);
if (target_size > 0)
target_size += tls_ctx->tx.overhead_size;
- trim_sg(sk, ctx->sg_encrypted_data,
- &ctx->sg_encrypted_num_elem,
- &ctx->sg_encrypted_size,
+ trim_sg(sk, rec->sg_encrypted_data,
+ &rec->sg_encrypted_num_elem,
+ &rec->sg_encrypted_size,
target_size);
}
@@ -265,15 +266,16 @@ static int alloc_encrypted_sg(struct sock *sk, int len)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec = ctx->open_rec;
int rc = 0;
rc = sk_alloc_sg(sk, len,
- ctx->sg_encrypted_data, 0,
- &ctx->sg_encrypted_num_elem,
- &ctx->sg_encrypted_size, 0);
+ rec->sg_encrypted_data, 0,
+ &rec->sg_encrypted_num_elem,
+ &rec->sg_encrypted_size, 0);
if (rc == -ENOSPC)
- ctx->sg_encrypted_num_elem = ARRAY_SIZE(ctx->sg_encrypted_data);
+ rec->sg_encrypted_num_elem = ARRAY_SIZE(rec->sg_encrypted_data);
return rc;
}
@@ -282,14 +284,15 @@ static int alloc_plaintext_sg(struct sock *sk, int len)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec = ctx->open_rec;
int rc = 0;
- rc = sk_alloc_sg(sk, len, ctx->sg_plaintext_data, 0,
- &ctx->sg_plaintext_num_elem, &ctx->sg_plaintext_size,
+ rc = sk_alloc_sg(sk, len, rec->sg_plaintext_data, 0,
+ &rec->sg_plaintext_num_elem, &rec->sg_plaintext_size,
tls_ctx->pending_open_record_frags);
if (rc == -ENOSPC)
- ctx->sg_plaintext_num_elem = ARRAY_SIZE(ctx->sg_plaintext_data);
+ rec->sg_plaintext_num_elem = ARRAY_SIZE(rec->sg_plaintext_data);
return rc;
}
@@ -311,37 +314,195 @@ static void tls_free_both_sg(struct sock *sk)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec = ctx->open_rec;
- free_sg(sk, ctx->sg_encrypted_data, &ctx->sg_encrypted_num_elem,
- &ctx->sg_encrypted_size);
+ /* Return if there is no open record */
+ if (!rec)
+ return;
+
+ free_sg(sk, rec->sg_encrypted_data,
+ &rec->sg_encrypted_num_elem,
+ &rec->sg_encrypted_size);
+
+ free_sg(sk, rec->sg_plaintext_data,
+ &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size);
+}
+
+static bool append_tx_ready_list(struct tls_context *tls_ctx,
+ struct tls_sw_context_tx *ctx,
+ struct tls_rec *enc_rec)
+{
+ u64 new_seq = be64_to_cpup((const __be64 *)&enc_rec->aad_space);
+ struct list_head *pos;
+
+ /* Need to insert encrypted record in tx_ready_list sorted
+ * as per sequence number. Traverse linked list from tail.
+ */
+ list_for_each_prev(pos, &ctx->tx_ready_list) {
+ struct tls_rec *rec = (struct tls_rec *)pos;
+ u64 seq = be64_to_cpup((const __be64 *)&rec->aad_space);
+
+ if (new_seq > seq)
+ break;
+ }
- free_sg(sk, ctx->sg_plaintext_data, &ctx->sg_plaintext_num_elem,
- &ctx->sg_plaintext_size);
+ list_add((struct list_head *)&enc_rec->list, pos);
+
+ return is_tx_ready(tls_ctx, ctx);
}
-static int tls_do_encryption(struct tls_context *tls_ctx,
+int tls_tx_records(struct sock *sk, int flags)
+{
+ struct tls_rec *rec, *tmp;
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ int rc = 0;
+ int tx_flags;
+
+ if (tls_is_partially_sent_record(tls_ctx)) {
+ rec = list_first_entry(&ctx->tx_ready_list,
+ struct tls_rec, list);
+
+ if (flags == -1)
+ tx_flags = rec->tx_flags;
+ else
+ tx_flags = flags;
+
+ rc = tls_push_partial_record(sk, tls_ctx, tx_flags);
+ if (rc)
+ goto tx_err;
+
+ /* Full record has been transmitted.
+ * Remove the head of tx_ready_list
+ */
+ tls_ctx->tx_seq_number++;
+ list_del(&rec->list);
+ kfree(rec);
+ }
+
+ /* Tx all ready records which have expected sequence number */
+ list_for_each_entry_safe(rec, tmp, &ctx->tx_ready_list, list) {
+ u64 seq;
+
+ seq = be64_to_cpup((const __be64 *)&rec->aad_space);
+
+ if (seq == tls_ctx->tx_seq_number) {
+ if (flags == -1)
+ tx_flags = rec->tx_flags;
+ else
+ tx_flags = flags;
+
+ rc = tls_push_sg(sk, tls_ctx,
+ &rec->sg_encrypted_data[0],
+ 0, tx_flags);
+ if (rc)
+ goto tx_err;
+
+ tls_ctx->tx_seq_number++;
+ list_del(&rec->list);
+ kfree(rec);
+ } else {
+ break;
+ }
+ }
+
+tx_err:
+ if (rc < 0 && rc != -EAGAIN)
+ tls_err_abort(sk, EBADMSG);
+
+ return rc;
+}
+
+static void tls_encrypt_done(struct crypto_async_request *req, int err)
+{
+ struct aead_request *aead_req = (struct aead_request *)req;
+ struct sock *sk = req->data;
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec;
+ int pending;
+ bool ready = false;
+
+ rec = container_of(aead_req, struct tls_rec, aead_req);
+
+ rec->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
+
+ free_sg(sk, rec->sg_plaintext_data,
+ &rec->sg_plaintext_num_elem, &rec->sg_plaintext_size);
+
+ /* Free the record if error is previously set on socket */
+ if (err || sk->sk_err) {
+ free_sg(sk, rec->sg_encrypted_data,
+ &rec->sg_encrypted_num_elem, &rec->sg_encrypted_size);
+
+ kfree(rec);
+ rec = NULL;
+
+ /* If err is already set on socket, return the same code */
+ if (sk->sk_err) {
+ ctx->async_wait.err = sk->sk_err;
+ } else {
+ ctx->async_wait.err = err;
+ tls_err_abort(sk, err);
+ }
+ }
+
+ /* Append the record in tx queue */
+ if (rec)
+ ready = append_tx_ready_list(tls_ctx, ctx, rec);
+
+ pending = atomic_dec_return(&ctx->encrypt_pending);
+
+ if (!pending && READ_ONCE(ctx->async_notify))
+ complete(&ctx->async_wait.completion);
+
+ if (!ready)
+ return;
+
+ /* Schedule the transmission */
+ if (!test_and_set_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
+ schedule_delayed_work(&ctx->tx_work.work, 1);
+}
+
+static int tls_do_encryption(struct sock *sk,
+ struct tls_context *tls_ctx,
struct tls_sw_context_tx *ctx,
struct aead_request *aead_req,
size_t data_len)
{
int rc;
+ struct tls_rec *rec = ctx->open_rec;
- ctx->sg_encrypted_data[0].offset += tls_ctx->tx.prepend_size;
- ctx->sg_encrypted_data[0].length -= tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[0].offset += tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[0].length -= tls_ctx->tx.prepend_size;
aead_request_set_tfm(aead_req, ctx->aead_send);
aead_request_set_ad(aead_req, TLS_AAD_SPACE_SIZE);
- aead_request_set_crypt(aead_req, ctx->sg_aead_in, ctx->sg_aead_out,
+ aead_request_set_crypt(aead_req, rec->sg_aead_in,
+ rec->sg_aead_out,
data_len, tls_ctx->tx.iv);
aead_request_set_callback(aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
- crypto_req_done, &ctx->async_wait);
+ tls_encrypt_done, sk);
- rc = crypto_wait_req(crypto_aead_encrypt(aead_req), &ctx->async_wait);
+ atomic_inc(&ctx->encrypt_pending);
- ctx->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
- ctx->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
+ rc = crypto_aead_encrypt(aead_req);
+ if (!rc || rc != -EINPROGRESS) {
+ atomic_dec(&ctx->encrypt_pending);
+ rec->sg_encrypted_data[0].offset -= tls_ctx->tx.prepend_size;
+ rec->sg_encrypted_data[0].length += tls_ctx->tx.prepend_size;
+ }
+
+ /* Case of encryption failure */
+ if (rc && rc != -EINPROGRESS)
+ return rc;
+ /* Unhook the record from context if encryption is not failure */
+ ctx->open_rec = NULL;
+ tls_advance_record_sn(sk, &tls_ctx->tx);
return rc;
}
@@ -350,53 +511,49 @@ static int tls_push_record(struct sock *sk, int flags,
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec = ctx->open_rec;
struct aead_request *req;
int rc;
- req = aead_request_alloc(ctx->aead_send, sk->sk_allocation);
- if (!req)
- return -ENOMEM;
+ if (!rec)
+ return 0;
+
+ rec->tx_flags = flags;
+ req = &rec->aead_req;
- sg_mark_end(ctx->sg_plaintext_data + ctx->sg_plaintext_num_elem - 1);
- sg_mark_end(ctx->sg_encrypted_data + ctx->sg_encrypted_num_elem - 1);
+ sg_mark_end(rec->sg_plaintext_data + rec->sg_plaintext_num_elem - 1);
+ sg_mark_end(rec->sg_encrypted_data + rec->sg_encrypted_num_elem - 1);
- tls_make_aad(ctx->aad_space, ctx->sg_plaintext_size,
+ tls_make_aad(rec->aad_space, rec->sg_plaintext_size,
tls_ctx->tx.rec_seq, tls_ctx->tx.rec_seq_size,
record_type);
tls_fill_prepend(tls_ctx,
- page_address(sg_page(&ctx->sg_encrypted_data[0])) +
- ctx->sg_encrypted_data[0].offset,
- ctx->sg_plaintext_size, record_type);
+ page_address(sg_page(&rec->sg_encrypted_data[0])) +
+ rec->sg_encrypted_data[0].offset,
+ rec->sg_plaintext_size, record_type);
tls_ctx->pending_open_record_frags = 0;
- set_bit(TLS_PENDING_CLOSED_RECORD, &tls_ctx->flags);
-
- rc = tls_do_encryption(tls_ctx, ctx, req, ctx->sg_plaintext_size);
- if (rc < 0) {
- /* If we are called from write_space and
- * we fail, we need to set this SOCK_NOSPACE
- * to trigger another write_space in the future.
- */
- set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
- goto out_req;
- }
- free_sg(sk, ctx->sg_plaintext_data, &ctx->sg_plaintext_num_elem,
- &ctx->sg_plaintext_size);
+ rc = tls_do_encryption(sk, tls_ctx, ctx, req, rec->sg_plaintext_size);
+ if (rc == -EINPROGRESS)
+ return -EINPROGRESS;
- ctx->sg_encrypted_num_elem = 0;
- ctx->sg_encrypted_size = 0;
+ free_sg(sk, rec->sg_plaintext_data, &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size);
- /* Only pass through MSG_DONTWAIT and MSG_NOSIGNAL flags */
- rc = tls_push_sg(sk, tls_ctx, ctx->sg_encrypted_data, 0, flags);
- if (rc < 0 && rc != -EAGAIN)
+ if (rc < 0) {
tls_err_abort(sk, EBADMSG);
+ return rc;
+ }
- tls_advance_record_sn(sk, &tls_ctx->tx);
-out_req:
- aead_request_free(req);
- return rc;
+ /* Put the record in tx_ready_list and start tx if permitted.
+ * This happens only when encryption is not asynchronous.
+ */
+ if (append_tx_ready_list(tls_ctx, ctx, rec))
+ return tls_tx_records(sk, flags);
+
+ return 0;
}
static int tls_sw_push_pending_record(struct sock *sk, int flags)
@@ -473,11 +630,12 @@ static int memcopy_from_iter(struct sock *sk, struct iov_iter *from,
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
- struct scatterlist *sg = ctx->sg_plaintext_data;
+ struct tls_rec *rec = ctx->open_rec;
+ struct scatterlist *sg = rec->sg_plaintext_data;
int copy, i, rc = 0;
for (i = tls_ctx->pending_open_record_frags;
- i < ctx->sg_plaintext_num_elem; ++i) {
+ i < rec->sg_plaintext_num_elem; ++i) {
copy = sg[i].length;
if (copy_from_iter(
page_address(sg_page(&sg[i])) + sg[i].offset,
@@ -497,6 +655,45 @@ static int memcopy_from_iter(struct sock *sk, struct iov_iter *from,
return rc;
}
+struct tls_rec *get_rec(struct sock *sk)
+{
+ int mem_size;
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec;
+
+ /* Return if we already have an open record */
+ if (ctx->open_rec)
+ return ctx->open_rec;
+
+ mem_size = sizeof(struct tls_rec) + crypto_aead_reqsize(ctx->aead_send);
+
+ rec = kzalloc(mem_size, sk->sk_allocation);
+ if (!rec)
+ return NULL;
+
+ sg_init_table(&rec->sg_plaintext_data[0],
+ ARRAY_SIZE(rec->sg_plaintext_data));
+ sg_init_table(&rec->sg_encrypted_data[0],
+ ARRAY_SIZE(rec->sg_encrypted_data));
+
+ sg_init_table(rec->sg_aead_in, 2);
+ sg_set_buf(&rec->sg_aead_in[0], rec->aad_space,
+ sizeof(rec->aad_space));
+ sg_unmark_end(&rec->sg_aead_in[1]);
+ sg_chain(rec->sg_aead_in, 2, rec->sg_plaintext_data);
+
+ sg_init_table(rec->sg_aead_out, 2);
+ sg_set_buf(&rec->sg_aead_out[0], rec->aad_space,
+ sizeof(rec->aad_space));
+ sg_unmark_end(&rec->sg_aead_out[1]);
+ sg_chain(rec->sg_aead_out, 2, rec->sg_encrypted_data);
+
+ ctx->open_rec = rec;
+
+ return rec;
+}
+
int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
@@ -510,21 +707,33 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
int record_room;
bool full_record;
int orig_size;
+ struct tls_rec *rec;
bool is_kvec = msg->msg_iter.type & ITER_KVEC;
+ struct crypto_tfm *tfm = crypto_aead_tfm(ctx->aead_send);
+ bool async_capable = tfm->__crt_alg->cra_flags & CRYPTO_ALG_ASYNC;
+ int num_async = 0;
+ int num_zc = 0;
if (msg->msg_flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL))
return -ENOTSUPP;
lock_sock(sk);
- ret = tls_complete_pending_work(sk, tls_ctx, msg->msg_flags, &timeo);
- if (ret)
- goto send_end;
+ /* Wait till there is any pending write on socket */
+ if (unlikely(sk->sk_write_pending)) {
+ ret = wait_on_pending_writer(sk, &timeo);
+ if (unlikely(ret))
+ goto send_end;
+ }
if (unlikely(msg->msg_controllen)) {
ret = tls_proccess_cmsg(sk, msg, &record_type);
- if (ret)
- goto send_end;
+ if (ret) {
+ if (ret == -EINPROGRESS)
+ num_async++;
+ else if (ret != -EAGAIN)
+ goto send_end;
+ }
}
while (msg_data_left(msg)) {
@@ -533,20 +742,27 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
goto send_end;
}
- orig_size = ctx->sg_plaintext_size;
+ rec = get_rec(sk);
+ if (!rec) {
+ ret = -ENOMEM;
+ goto send_end;
+ }
+
+ orig_size = rec->sg_plaintext_size;
full_record = false;
try_to_copy = msg_data_left(msg);
- record_room = TLS_MAX_PAYLOAD_SIZE - ctx->sg_plaintext_size;
+ record_room = TLS_MAX_PAYLOAD_SIZE - rec->sg_plaintext_size;
if (try_to_copy >= record_room) {
try_to_copy = record_room;
full_record = true;
}
- required_size = ctx->sg_plaintext_size + try_to_copy +
+ required_size = rec->sg_plaintext_size + try_to_copy +
tls_ctx->tx.overhead_size;
if (!sk_stream_memory_free(sk))
goto wait_for_sndbuf;
+
alloc_encrypted:
ret = alloc_encrypted_sg(sk, required_size);
if (ret) {
@@ -557,33 +773,39 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
* actually allocated. The difference is due
* to max sg elements limit
*/
- try_to_copy -= required_size - ctx->sg_encrypted_size;
+ try_to_copy -= required_size - rec->sg_encrypted_size;
full_record = true;
}
- if (!is_kvec && (full_record || eor)) {
+
+ if (!is_kvec && (full_record || eor) && !async_capable) {
ret = zerocopy_from_iter(sk, &msg->msg_iter,
- try_to_copy, &ctx->sg_plaintext_num_elem,
- &ctx->sg_plaintext_size,
- ctx->sg_plaintext_data,
- ARRAY_SIZE(ctx->sg_plaintext_data),
+ try_to_copy, &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size,
+ rec->sg_plaintext_data,
+ ARRAY_SIZE(rec->sg_plaintext_data),
true);
if (ret)
goto fallback_to_reg_send;
+ num_zc++;
copied += try_to_copy;
ret = tls_push_record(sk, msg->msg_flags, record_type);
- if (ret)
- goto send_end;
+ if (ret) {
+ if (ret == -EINPROGRESS)
+ num_async++;
+ else if (ret != -EAGAIN)
+ goto send_end;
+ }
continue;
fallback_to_reg_send:
- trim_sg(sk, ctx->sg_plaintext_data,
- &ctx->sg_plaintext_num_elem,
- &ctx->sg_plaintext_size,
+ trim_sg(sk, rec->sg_plaintext_data,
+ &rec->sg_plaintext_num_elem,
+ &rec->sg_plaintext_size,
orig_size);
}
- required_size = ctx->sg_plaintext_size + try_to_copy;
+ required_size = rec->sg_plaintext_size + try_to_copy;
alloc_plaintext:
ret = alloc_plaintext_sg(sk, required_size);
if (ret) {
@@ -594,13 +816,13 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
* actually allocated. The difference is due
* to max sg elements limit
*/
- try_to_copy -= required_size - ctx->sg_plaintext_size;
+ try_to_copy -= required_size - rec->sg_plaintext_size;
full_record = true;
- trim_sg(sk, ctx->sg_encrypted_data,
- &ctx->sg_encrypted_num_elem,
- &ctx->sg_encrypted_size,
- ctx->sg_plaintext_size +
+ trim_sg(sk, rec->sg_encrypted_data,
+ &rec->sg_encrypted_num_elem,
+ &rec->sg_encrypted_size,
+ rec->sg_plaintext_size +
tls_ctx->tx.overhead_size);
}
@@ -610,13 +832,12 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
copied += try_to_copy;
if (full_record || eor) {
-push_record:
ret = tls_push_record(sk, msg->msg_flags, record_type);
if (ret) {
- if (ret == -ENOMEM)
- goto wait_for_memory;
-
- goto send_end;
+ if (ret == -EINPROGRESS)
+ num_async++;
+ else if (ret != -EAGAIN)
+ goto send_end;
}
}
@@ -632,15 +853,37 @@ int tls_sw_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
goto send_end;
}
- if (tls_is_pending_closed_record(tls_ctx))
- goto push_record;
-
- if (ctx->sg_encrypted_size < required_size)
+ if (rec->sg_encrypted_size < required_size)
goto alloc_encrypted;
goto alloc_plaintext;
}
+ if (!num_async) {
+ goto send_end;
+ } else if (num_zc) {
+ /* Wait for pending encryptions to get completed */
+ smp_store_mb(ctx->async_notify, true);
+
+ if (atomic_read(&ctx->encrypt_pending))
+ crypto_wait_req(-EINPROGRESS, &ctx->async_wait);
+ else
+ reinit_completion(&ctx->async_wait.completion);
+
+ WRITE_ONCE(ctx->async_notify, false);
+
+ if (ctx->async_wait.err) {
+ ret = ctx->async_wait.err;
+ copied = 0;
+ }
+ }
+
+ /* Transmit if any encryptions have completed */
+ if (test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask)) {
+ cancel_delayed_work(&ctx->tx_work.work);
+ tls_tx_records(sk, msg->msg_flags);
+ }
+
send_end:
ret = sk_stream_error(sk, msg->msg_flags, ret);
@@ -661,6 +904,8 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
struct scatterlist *sg;
bool full_record;
int record_room;
+ struct tls_rec *rec;
+ int num_async = 0;
if (flags & ~(MSG_MORE | MSG_DONTWAIT | MSG_NOSIGNAL |
MSG_SENDPAGE_NOTLAST))
@@ -673,9 +918,12 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
- ret = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
- if (ret)
- goto sendpage_end;
+ /* Wait till there is any pending write on socket */
+ if (unlikely(sk->sk_write_pending)) {
+ ret = wait_on_pending_writer(sk, &timeo);
+ if (unlikely(ret))
+ goto sendpage_end;
+ }
/* Call the sk_stream functions to manage the sndbuf mem. */
while (size > 0) {
@@ -686,14 +934,20 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
goto sendpage_end;
}
+ rec = get_rec(sk);
+ if (!rec) {
+ ret = -ENOMEM;
+ goto sendpage_end;
+ }
+
full_record = false;
- record_room = TLS_MAX_PAYLOAD_SIZE - ctx->sg_plaintext_size;
+ record_room = TLS_MAX_PAYLOAD_SIZE - rec->sg_plaintext_size;
copy = size;
if (copy >= record_room) {
copy = record_room;
full_record = true;
}
- required_size = ctx->sg_plaintext_size + copy +
+ required_size = rec->sg_plaintext_size + copy +
tls_ctx->tx.overhead_size;
if (!sk_stream_memory_free(sk))
@@ -708,33 +962,32 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
* actually allocated. The difference is due
* to max sg elements limit
*/
- copy -= required_size - ctx->sg_plaintext_size;
+ copy -= required_size - rec->sg_plaintext_size;
full_record = true;
}
get_page(page);
- sg = ctx->sg_plaintext_data + ctx->sg_plaintext_num_elem;
+ sg = rec->sg_plaintext_data + rec->sg_plaintext_num_elem;
sg_set_page(sg, page, copy, offset);
sg_unmark_end(sg);
- ctx->sg_plaintext_num_elem++;
+ rec->sg_plaintext_num_elem++;
sk_mem_charge(sk, copy);
offset += copy;
size -= copy;
- ctx->sg_plaintext_size += copy;
- tls_ctx->pending_open_record_frags = ctx->sg_plaintext_num_elem;
+ rec->sg_plaintext_size += copy;
+ tls_ctx->pending_open_record_frags = rec->sg_plaintext_num_elem;
if (full_record || eor ||
- ctx->sg_plaintext_num_elem ==
- ARRAY_SIZE(ctx->sg_plaintext_data)) {
-push_record:
+ rec->sg_plaintext_num_elem ==
+ ARRAY_SIZE(rec->sg_plaintext_data)) {
ret = tls_push_record(sk, flags, record_type);
if (ret) {
- if (ret == -ENOMEM)
- goto wait_for_memory;
-
- goto sendpage_end;
+ if (ret == -EINPROGRESS)
+ num_async++;
+ else if (ret != -EAGAIN)
+ goto sendpage_end;
}
}
continue;
@@ -743,16 +996,20 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
wait_for_memory:
ret = sk_stream_wait_memory(sk, &timeo);
if (ret) {
- trim_both_sgl(sk, ctx->sg_plaintext_size);
+ trim_both_sgl(sk, rec->sg_plaintext_size);
goto sendpage_end;
}
- if (tls_is_pending_closed_record(tls_ctx))
- goto push_record;
-
goto alloc_payload;
}
+ if (num_async) {
+ /* Transmit if any encryptions have completed */
+ if (test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask)) {
+ cancel_delayed_work(&ctx->tx_work.work);
+ tls_tx_records(sk, flags);
+ }
+ }
sendpage_end:
if (orig_size > size)
ret = orig_size - size;
@@ -1300,6 +1557,49 @@ void tls_sw_free_resources_tx(struct sock *sk)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+ struct tls_rec *rec, *tmp;
+
+ /* Wait for any pending async encryptions to complete */
+ smp_store_mb(ctx->async_notify, true);
+ if (atomic_read(&ctx->encrypt_pending))
+ crypto_wait_req(-EINPROGRESS, &ctx->async_wait);
+
+ cancel_delayed_work_sync(&ctx->tx_work.work);
+
+ /* Tx whatever records we can transmit and abandon the rest */
+ tls_tx_records(sk, -1);
+
+ /* Free up un-sent records in tx_ready_list. First, free
+ * the partially sent record if any at head of tx_list.
+ */
+ if (tls_ctx->partially_sent_record) {
+ struct scatterlist *sg = tls_ctx->partially_sent_record;
+
+ while (1) {
+ put_page(sg_page(sg));
+ sk_mem_uncharge(sk, sg->length);
+
+ if (sg_is_last(sg))
+ break;
+ sg++;
+ }
+
+ tls_ctx->partially_sent_record = NULL;
+
+ rec = list_first_entry(&ctx->tx_ready_list,
+ struct tls_rec, list);
+ list_del(&rec->list);
+ kfree(rec);
+ }
+
+ list_for_each_entry_safe(rec, tmp, &ctx->tx_ready_list, list) {
+ free_sg(sk, rec->sg_encrypted_data,
+ &rec->sg_encrypted_num_elem,
+ &rec->sg_encrypted_size);
+
+ list_del(&rec->list);
+ kfree(rec);
+ }
crypto_free_aead(ctx->aead_send);
tls_free_both_sg(sk);
@@ -1336,6 +1636,24 @@ void tls_sw_free_resources_rx(struct sock *sk)
kfree(ctx);
}
+/* The work handler to transmitt the encrypted records in tx_ready_list */
+static void tx_work_handler(struct work_struct *work)
+{
+ struct delayed_work *delayed_work = to_delayed_work(work);
+ struct tx_work *tx_work = container_of(delayed_work,
+ struct tx_work, work);
+ struct sock *sk = tx_work->sk;
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_sw_context_tx *ctx = tls_sw_ctx_tx(tls_ctx);
+
+ if (!test_and_clear_bit(BIT_TX_SCHEDULED, &ctx->tx_bitmask))
+ return;
+
+ lock_sock(sk);
+ tls_tx_records(sk, -1);
+ release_sock(sk);
+}
+
int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
{
struct tls_crypto_info *crypto_info;
@@ -1385,6 +1703,9 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
crypto_info = &ctx->crypto_send.info;
cctx = &ctx->tx;
aead = &sw_ctx_tx->aead_send;
+ INIT_LIST_HEAD(&sw_ctx_tx->tx_ready_list);
+ INIT_DELAYED_WORK(&sw_ctx_tx->tx_work.work, tx_work_handler);
+ sw_ctx_tx->tx_work.sk = sk;
} else {
crypto_init_wait(&sw_ctx_rx->async_wait);
crypto_info = &ctx->crypto_recv.info;
@@ -1435,26 +1756,6 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
goto free_iv;
}
- if (sw_ctx_tx) {
- 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) {
*aead = crypto_alloc_aead("gcm(aes)", 0, 0);
if (IS_ERR(*aead)) {
@@ -1491,6 +1792,8 @@ int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx, int tx)
sw_ctx_rx->sk_poll = sk->sk_socket->ops->poll;
strp_check_rcv(&sw_ctx_rx->strp);
+ } else {
+ ctx->tx_seq_number = be64_to_cpup((const __be64 *)rec_seq);
}
goto out;
--
2.13.6
^ permalink raw reply related
* Re: [PATCH 1/1] macsec: reflect the master interface state
From: Sabrina Dubroca @ 2018-09-19 15:24 UTC (permalink / raw)
To: Radu Rendec; +Cc: netdev, David Miller, Patrick Talbert
In-Reply-To: <20180919001612.2636-2-radu.rendec@gmail.com>
Hello,
2018-09-18, 20:16:12 -0400, Radu Rendec wrote:
> This patch makes macsec interfaces reflect the state of the underlying
> interface: if the master interface changes state to up/down, the macsec
> interface changes its state accordingly.
I got a separate report of the same issue and I've been looking in
that area too.
> This closes a loophole in the macsec interface state logic: the macsec
> interface cannot be brought up if the master interface is down (the
> operation is rejected with ENETDOWN); however, if the macsec interface
> is brought up while the master interface is up and then the master
> interface goes down, the macsec interface stays up.
Yes, that's a bit unfortunate. I was wondering if we should just allow
setting the device up, and let link state handle the rest.
> Reflecting the master interface state can also be useful if the macsec
> interface is used as a bridge port: if the master (physical) interface
> goes down, the state propagates through the macsec interface to the
> bridge module, which can react to the state change (for example if it
> runs STP).
>
> The patch does nothing original. The same logic is implemented for vlan
> interfaces in vlan_device_event() in net/8021q/vlan.c. In fact, the code
> was copied and adapted from there.
It's missing small parts of link state handling that I have been
testing, mainly when creating a new device.
> Signed-off-by: Radu Rendec <radu.rendec@gmail.com>
> ---
> drivers/net/macsec.c | 57 +++++++++++++++++++++++++++++++++++---------
> 1 file changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
> index 7de88b33d5b9..cb93a1290f85 100644
> --- a/drivers/net/macsec.c
> +++ b/drivers/net/macsec.c
> @@ -3486,20 +3486,21 @@ static int macsec_notify(struct notifier_block *this, unsigned long event,
> void *ptr)
> {
> struct net_device *real_dev = netdev_notifier_info_to_dev(ptr);
> - LIST_HEAD(head);
> + struct macsec_dev *m;
> + struct macsec_rxh_data *rxd;
>
> if (!is_macsec_master(real_dev))
> return NOTIFY_DONE;
>
> + rxd = macsec_data_rtnl(real_dev);
> +
> switch (event) {
> case NETDEV_UNREGISTER: {
> - struct macsec_dev *m, *n;
> - struct macsec_rxh_data *rxd;
> + struct macsec_dev *n;
> + LIST_HEAD(head);
>
> - rxd = macsec_data_rtnl(real_dev);
> - list_for_each_entry_safe(m, n, &rxd->secys, secys) {
> + list_for_each_entry_safe(m, n, &rxd->secys, secys)
> macsec_common_dellink(m->secy.netdev, &head);
> - }
Please don't mix coding style changes with bug fixes.
[...]
> + case NETDEV_DOWN: {
> + struct net_device *dev, *tmp;
> + LIST_HEAD(close_list);
> +
> + list_for_each_entry(m, &rxd->secys, secys) {
> + dev = m->secy.netdev;
> +
> + if (dev->flags & IFF_UP)
> + list_add(&dev->close_list, &close_list);
> + }
> +
> + dev_close_many(&close_list, false);
> +
> + list_for_each_entry_safe(dev, tmp, &close_list, close_list) {
> + netif_stacked_transfer_operstate(real_dev, dev);
> + list_del_init(&dev->close_list);
> + }
> + list_del(&close_list);
> + break;
My version of the patch only does netif_stacked_transfer_operstate(),
and skips setting the device administratively down (ie, same as
NETDEV_CHANGE).
> }
> + case NETDEV_UP:
> + list_for_each_entry(m, &rxd->secys, secys) {
> + struct net_device *dev = m->secy.netdev;
> + int flags = dev_get_flags(dev);
> +
> + if (!(flags & IFF_UP)) {
> + dev_change_flags(dev, flags | IFF_UP);
> + netif_stacked_transfer_operstate(real_dev, dev);
> + }
> + }
> + break;
I don't like that this completely ignores whether the device was done
independently of the lower link. Maybe the administrator actually
wants the macsec device down, regardless of state changes on the lower
device.
I know it's consistent with what vlan is doing, but I'm not convinced
macsec should adopt this behavior.
> }
>
> return NOTIFY_OK;
> --
> 2.17.1
>
--
Sabrina
^ permalink raw reply
* Re: [RFC bpf-next 3/4] tools/bpf: bpftool, split the function do_dump()
From: Jakub Kicinski @ 2018-09-19 15:26 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
David S . Miller, netdev
In-Reply-To: <20180919075143.9308-4-bhole_prashant_q7@lab.ntt.co.jp>
On Wed, 19 Sep 2018 16:51:42 +0900, Prashant Bhole wrote:
> +static int dump_map_elem(int fd, void *key, void *value,
> + struct bpf_map_info *map_info, struct btf *btf,
> + json_writer_t *btf_wtr)
> +{
> + int num_elems = 0;
> +
> + if (!bpf_map_lookup_elem(fd, key, value)) {
> + if (json_output) {
> + print_entry_json(map_info, key, value, btf);
> + } else {
> + if (btf) {
> + struct btf_dumper d = {
> + .btf = btf,
> + .jw = btf_wtr,
> + .is_plain_text = true,
> + };
> +
> + do_dump_btf(&d, map_info, key, value);
> + } else {
> + print_entry_plain(map_info, key, value);
> + }
> + num_elems++;
> + }
> + goto out;
> + }
> +
> + /* lookup error handling */
> + if (map_is_map_of_maps(map_info->type) ||
> + map_is_map_of_progs(map_info->type))
> + goto out;
> +
nit: why not just return? the goto seems to only do a return anyway,
is this suggested by some coding style? Is it to help the
compiler? I see people do this from time to time..
[...]
> +out:
> + return num_elems;
^ permalink raw reply
* Re: [PATCH] ixgbevf: off by one in ixgbevf_ipsec_tx()
From: Shannon Nelson @ 2018-09-19 15:28 UTC (permalink / raw)
To: Dan Carpenter, Jeff Kirsher
Cc: David S. Miller, intel-wired-lan, netdev, kernel-janitors
In-Reply-To: <20180919103529.GC9238@mwanda>
On 9/19/2018 3:35 AM, Dan Carpenter wrote:
> The ipsec->tx_tbl[] array has IXGBE_IPSEC_MAX_SA_COUNT elements so the >
> should be a >=.
>
> Fixes: 0062e7cc955e ("ixgbevf: add VF IPsec offload code")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Shannon Nelson <shannon.nelson@oracle.com>
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/ipsec.c b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> index 997cea675a37..4fcbeffce52b 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/ipsec.c
> @@ -470,7 +470,7 @@ int ixgbevf_ipsec_tx(struct ixgbevf_ring *tx_ring,
> }
>
> sa_idx = xs->xso.offload_handle - IXGBE_IPSEC_BASE_TX_INDEX;
> - if (unlikely(sa_idx > IXGBE_IPSEC_MAX_SA_COUNT)) {
> + if (unlikely(sa_idx >= IXGBE_IPSEC_MAX_SA_COUNT)) {
> netdev_err(tx_ring->netdev, "%s: bad sa_idx=%d handle=%lu\n",
> __func__, sa_idx, xs->xso.offload_handle);
> return 0;
>
^ permalink raw reply
* Re: [RFC bpf-next 4/4] tools/bpf: handle EOPNOTSUPP when map lookup is failed
From: Jakub Kicinski @ 2018-09-19 15:29 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
David S . Miller, netdev
In-Reply-To: <20180919075143.9308-5-bhole_prashant_q7@lab.ntt.co.jp>
On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
> Let's add a check for EOPNOTSUPP error when map lookup is failed.
> Also in case map doesn't support lookup, the output of map dump is
> changed from "can't lookup element" to "lookup not supported for
> this map".
>
> Patch adds function print_entry_error() function to print the error
> value.
>
> Following example dumps a map which does not support lookup.
>
> Output before:
> root# bpftool map -jp dump id 40
> [
> "key": ["0x0a","0x00","0x00","0x00"
> ],
> "value": {
> "error": "can\'t lookup element"
> },
> "key": ["0x0b","0x00","0x00","0x00"
> ],
> "value": {
> "error": "can\'t lookup element"
> }
> ]
>
> root# bpftool map dump id 40
> can't lookup element with key:
> 0a 00 00 00
> can't lookup element with key:
> 0b 00 00 00
> Found 0 elements
>
> Output after changes:
> root# bpftool map dump -jp id 45
> [
> "key": ["0x0a","0x00","0x00","0x00"
> ],
> "value": {
> "error": "lookup not supported for this map"
> },
> "key": ["0x0b","0x00","0x00","0x00"
> ],
> "value": {
> "error": "lookup not supported for this map"
> }
> ]
>
> root# bpftool map dump id 45
> key:
> 0a 00 00 00
> value:
> lookup not supported for this map
> key:
> 0b 00 00 00
> value:
> lookup not supported for this map
> Found 0 elements
Nice improvement, thanks for the changes! I wonder what your thoughts
would be on just printing some form of "lookup not supported for this
map" only once? It seems slightly like repeated information - if
lookup is not supported for one key it likely won't be for other keys
too, so we could shorten the output. Would that make sense?
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> tools/bpf/bpftool/main.h | 5 +++++
> tools/bpf/bpftool/map.c | 35 ++++++++++++++++++++++++++++++-----
> 2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 40492cdc4e53..1a8c683f949b 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -46,6 +46,11 @@
>
> #include "json_writer.h"
>
> +#define ERR_CANNOT_LOOKUP \
> + "can't lookup element"
> +#define ERR_LOOKUP_NOT_SUPPORTED \
> + "lookup not supported for this map"
Do we need these? Are we going to reused them in more parts of the
code?
> #define ptr_to_u64(ptr) ((__u64)(unsigned long)(ptr))
>
> #define NEXT_ARG() ({ argc--; argv++; if (argc < 0) usage(); })
> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
> index 284e12a289c0..2faccd2098c9 100644
> --- a/tools/bpf/bpftool/map.c
> +++ b/tools/bpf/bpftool/map.c
> @@ -333,6 +333,25 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
> jsonw_end_object(json_wtr);
> }
>
> +static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
> + char *value)
> +{
> + bool single_line, break_names;
> + int value_size = strlen(value);
nit: order variables declaration lines to shortest, please.
> +
> + break_names = info->key_size > 16 || value_size > 16;
> + single_line = info->key_size + value_size <= 24 && !break_names;
> +
> + printf("key:%c", break_names ? '\n' : ' ');
> + fprint_hex(stdout, key, info->key_size, " ");
> +
> + printf(single_line ? " " : "\n");
> +
> + printf("value:%c%s", break_names ? '\n' : ' ', value);
> +
> + printf("\n");
> +}
> +
> static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
> unsigned char *value)
> {
> @@ -660,6 +679,8 @@ static int dump_map_elem(int fd, void *key, void *value,
> json_writer_t *btf_wtr)
> {
> int num_elems = 0;
> + int lookup_errno;
> + char *errstr;
nit: const char?
>
> if (!bpf_map_lookup_elem(fd, key, value)) {
> if (json_output) {
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Andrew Lunn @ 2018-09-19 15:33 UTC (permalink / raw)
To: Michal Kubecek
Cc: Edward Cree, John W. Linville, netdev, Ganesh Goudar,
Jakub Kicinski, Dustin Byford, Dirk van der Merwe
In-Reply-To: <20180919144117.GF3876@unicorn.suse.cz>
> One loosely related question: how sure can we be that we are never going
> to need more than 32 bits for FEC encodings? Is it something completely
> hypothetical or is it something that could happen in the future?
>
Hi Michal
Hopefully we have moved to a netlink socket by that time :-)
I recently found that EEE still uses a u32 for advertise link modes.
We should fix that in the netlink API.
Andrew
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Edward Cree @ 2018-09-19 15:38 UTC (permalink / raw)
To: Michal Kubecek
Cc: John W. Linville, netdev, Ganesh Goudar, Jakub Kicinski,
Dustin Byford, Dirk van der Merwe
In-Reply-To: <20180919144117.GF3876@unicorn.suse.cz>
On 19/09/18 15:41, Michal Kubecek wrote:
> I'm sorry I didn't notice this before the patch was accepted but as it's
> not in a release yet, maybe it's still not too late.
>
> Could I suggest to make the syntax consistent with other options?
I didn't realise ethtool had any patterns to be consistent with ;)
> I mean rather than a comma separated list to use either
>
> ethtool --set-fec <dev> encoding enc1 enc2 ...
but yes this looks fine to me, as long as we're reasonably confident that
we won't want to add new parameters (that might require determining
whether enc2 is an encoding or a parameter name) in the future, because
while the parsing wouldn't be impossible it might get ugly.
I'll rustle up an RFC patch.
-Ed
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Michal Kubecek @ 2018-09-19 15:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: Edward Cree, John W. Linville, netdev, Ganesh Goudar,
Jakub Kicinski, Dustin Byford, Dirk van der Merwe
In-Reply-To: <20180919153338.GE17466@lunn.ch>
On Wed, Sep 19, 2018 at 05:33:38PM +0200, Andrew Lunn wrote:
> > One loosely related question: how sure can we be that we are never going
> > to need more than 32 bits for FEC encodings? Is it something completely
> > hypothetical or is it something that could happen in the future?
> >
> Hi Michal
>
> Hopefully we have moved to a netlink socket by that time :-)
Actually, that's why I'm asking. :-)
> I recently found that EEE still uses a u32 for advertise link modes.
> We should fix that in the netlink API.
For EEE it felt obvious that arbitrary length bitmap is the way to go so
I used it (it's still u32 in ethtool_ops but that's easier to change
when needed).
For FEC I wasn't sure if it wouldn't be an overkill.
Michal Kubecek
^ permalink raw reply
* Re: [RFC] net;sched: Try to find idle cpu for RPS to handle packets
From: Eric Dumazet @ 2018-09-19 15:49 UTC (permalink / raw)
To: Kirill Tkhai
Cc: Peter Zijlstra, David Miller, Daniel Borkmann, tom, netdev, LKML
In-Reply-To: <2fdf2bd7-1cc4-a1e1-15c2-e2badfcd4d59@virtuozzo.com>
[-- Attachment #1: Type: text/plain, Size: 4801 bytes --]
On Wed, Sep 19, 2018 at 8:41 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>
> On 19.09.2018 17:55, Eric Dumazet wrote:
> > On Wed, Sep 19, 2018 at 5:29 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> >>
> >> Many workloads have polling mode of work. The application
> >> checks for incomming packets from time to time, but it also
> >> has a work to do, when there is no packets. This RFC
> >> tries to develop an idea to queue RPS packets on idle
> >> CPU in the the L3 domain of the consumer, so backlog
> >> processing of the packets and the application can execute
> >> in parallel.
> >>
> >> We require this in case of network cards does not
> >> have enough RX queues to cover all online CPUs (this seems
> >> to be the most cards), and get_rps_cpu() actually chooses
> >> remote cpu, and SMP interrupt is sent. Here we may try
> >> our best, and to find idle CPU nearly the consumer's CPU.
> >> Note, that in case of consumer works in poll mode and it
> >> does not waits for incomming packets, its CPU will be not
> >> idle, while CPU of a sleeping consumer may be idle. So,
> >> not polling consumers will still be able to have skb
> >> handled on its CPU.
> >>
> >> In case of network card has many queues, the device
> >> interrupts will come on consumer's CPU, and this patch
> >> won't try to find idle cpu for them.
> >>
> >> I've tried simple netperf test for this:
> >> netserver -p 1234
> >> netperf -L 127.0.0.1 -p 1234 -l 100
> >>
> >> Before:
> >> 87380 16384 16384 100.00 60323.56
> >> 87380 16384 16384 100.00 60388.46
> >> 87380 16384 16384 100.00 60217.68
> >> 87380 16384 16384 100.00 57995.41
> >> 87380 16384 16384 100.00 60659.00
> >>
> >> After:
> >> 87380 16384 16384 100.00 64569.09
> >> 87380 16384 16384 100.00 64569.25
> >> 87380 16384 16384 100.00 64691.63
> >> 87380 16384 16384 100.00 64930.14
> >> 87380 16384 16384 100.00 62670.15
> >>
> >> The difference between best runs is +7%,
> >> the worst runs differ +8%.
> >>
> >> What do you think about following somehow in this way?
> >
> > Hi Kirill
> >
> > In my experience, scheduler has a poor view of softirq processing
> > happening on various cpus.
> > A cpu spending 90% of its cycles processing IRQ might be considered 'idle'
>
> Yes, in case of there is softirq on top of irq_exit(), the cpu is not
> considered as busy. But after MAX_SOFTIRQ_TIME (=2ms), ksoftirqd are
> waken up to execute the work in process context, and the processor is
> considered as !idle. 2ms is 2 timer ticks in case of HZ=1000. So, we
> don't restart softirq in case of it was executed for more then 2ms.
>
That's the theory, but reality is very different unfortunately.
If RFS/RPS is setup properly, we really do not hit MAX_SOFTIRQ_TIME condition
unless in some synthetic benchmarks maybe.
> The similar way, single net_rx_action() can't be executed longer
> than 2ms.
>
> Having 90% load in softirq (called on top of irq_exit()) should be
> very unlikely situation, when there are too many interrupts with small
> amount of work, which related softirq calls are doing for each of them.
> I think it had be a problem even in plain napi case, since it would
> worked not like expected.
>
> But anyway. You worry, that during handling of next portion of skbs,
> we find that previous portion of skbs already woken ksoftirqd, and
> we don't see this cpu as idle? Yeah, then we'll try to change cpu,
> and this is not what we want. We want to continue use the cpu, where
> previous portion was handler. Hm, not so fast I'll answer, but certainly,
> this may be handled somehow in more creative way.
>
> > So please run a real workload (it is _very_ uncommon anyone set up RPS
> > on lo interface !)
> >
> > Like 400 or more concurrent netperf -t TCP_RR on a 10Gbit NIC.
>
> Yeah, it's just a simulation of a single irq nic. I'll try on something
> more real hardware.
Also my concern is that you might have results that are tied to a particular
version of process scheduling, platform, workload...
One month later, a small change in process scheduler,
and very different results.
This is why I believe this new feature must be controllable, via a new
tunable (like RPS/RFS are controllable per rx queue)
>
> How do you execute such the tests? I don't see the appropriate parameter
> of netperf. Does this mean just to start 400 copies of netperf? How is
> to aggregate their results in this case?
Yeah, there are various 'super_netperf' scripts available on the net
(almost trivial to write anyway)
( I am attaching one of them)
Thanks.
>
> > Thanks.
> >
> > PS: Idea of playing with L3 domains is interesting, I have personally
> > tried various strategies in the past but none of them
> > demonstrated a clear win.
>
> Thanks,
> Kirill
[-- Attachment #2: super_netperf --]
[-- Type: application/octet-stream, Size: 435 bytes --]
#!/bin/bash
run_netperf() {
loops=$1
shift
for ((i=0; i<loops; i++)); do
./netperf -s 2 $@ | awk '/Min/{
if (!once) {
print;
once=1;
}
}
{
if (NR == 6)
save = $NF
else if (NR==7) {
if (NF > 0)
print $NF
else
print save
} else if (NR==11) {
print $0
}
}' &
done
wait
return 0
}
run_netperf $@ | awk '{if (NF==7) {print $0; next}} {sum += $1} END {printf "%7u\n",sum}'
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Michal Kubecek @ 2018-09-19 15:56 UTC (permalink / raw)
To: Edward Cree
Cc: John W. Linville, netdev, Ganesh Goudar, Jakub Kicinski,
Dustin Byford, Dirk van der Merwe
In-Reply-To: <613d954c-d88c-fa20-f436-75625f11a2ae@solarflare.com>
On Wed, Sep 19, 2018 at 04:38:27PM +0100, Edward Cree wrote:
> On 19/09/18 15:41, Michal Kubecek wrote:
> > I'm sorry I didn't notice this before the patch was accepted but as it's
> > not in a release yet, maybe it's still not too late.
> >
> > Could I suggest to make the syntax consistent with other options?
> I didn't realise ethtool had any patterns to be consistent with ;)
Way too many, I must say. :-) That is why I wasn't happy about adding
another.
> > I mean rather than a comma separated list to use either
> >
> > ethtool --set-fec <dev> encoding enc1 enc2 ...
> but yes this looks fine to me, as long as we're reasonably confident that
> we won't want to add new parameters (that might require determining
> whether enc2 is an encoding or a parameter name) in the future, because
> while the parsing wouldn't be impossible it might get ugly.
This problem already exists for "-s ... msglvl". In the parser for the
netlink series I introduced an "end of list" marker (tentatively "--")
for this purpose, perhaps that could be a way.
> I'll rustle up an RFC patch.
Thank you.
Michal Kubecek
^ permalink raw reply
* [RFC PATCH ethtool] ethtool: better syntax for combinations of FEC modes
From: Edward Cree @ 2018-09-19 16:06 UTC (permalink / raw)
To: John W. Linville; +Cc: Michal Kubecek, netdev
In-Reply-To: <20180919144117.GF3876@unicorn.suse.cz>
Instead of commas, just have them as separate argvs.
The parsing state machine might look heavyweight, but it makes it easy to add
more parameters later and distinguish parameter names from encoding names.
Suggested-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
ethtool.8.in | 6 +++---
ethtool.c | 63 ++++++++++++++++------------------------------------------
test-cmdline.c | 10 +++++-----
3 files changed, 25 insertions(+), 54 deletions(-)
diff --git a/ethtool.8.in b/ethtool.8.in
index 414eaa1..7ea2cc0 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -390,7 +390,7 @@ ethtool \- query or control network driver and hardware settings
.B ethtool \-\-set\-fec
.I devname
.B encoding
-.BR auto | off | rs | baser [ , ...]
+.BR auto | off | rs | baser \ [...]
.
.\" Adjust lines (i.e. full justification) and hyphenate.
.ad
@@ -1120,11 +1120,11 @@ current FEC mode, the driver or firmware must take the link down
administratively and report the problem in the system logs for users to correct.
.RS 4
.TP
-.BR encoding\ auto | off | rs | baser [ , ...]
+.BR encoding\ auto | off | rs | baser \ [...]
Sets the FEC encoding for the device. Combinations of options are specified as
e.g.
-.B auto,rs
+.B encoding auto rs
; the semantics of such combinations vary between drivers.
.TS
nokeep;
diff --git a/ethtool.c b/ethtool.c
index 9997930..2f7e96b 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4979,39 +4979,6 @@ static int fecmode_str_to_type(const char *str)
return 0;
}
-/* Takes a comma-separated list of FEC modes, returns the bitwise OR of their
- * corresponding ETHTOOL_FEC_* constants.
- * Accepts repetitions (e.g. 'auto,auto') and trailing comma (e.g. 'off,').
- */
-static int parse_fecmode(const char *str)
-{
- int fecmode = 0;
- char buf[6];
-
- if (!str)
- return 0;
- while (*str) {
- size_t next;
- int mode;
-
- next = strcspn(str, ",");
- if (next >= 6) /* Bad mode, longest name is 5 chars */
- return 0;
- /* Copy into temp buffer and nul-terminate */
- memcpy(buf, str, next);
- buf[next] = 0;
- mode = fecmode_str_to_type(buf);
- if (!mode) /* Bad mode encountered */
- return 0;
- fecmode |= mode;
- str += next;
- /* Skip over ',' (but not nul) */
- if (*str)
- str++;
- }
- return fecmode;
-}
-
static int do_gfec(struct cmd_context *ctx)
{
struct ethtool_fecparam feccmd = { 0 };
@@ -5041,22 +5008,26 @@ static int do_gfec(struct cmd_context *ctx)
static int do_sfec(struct cmd_context *ctx)
{
- char *fecmode_str = NULL;
+ enum { ARG_NONE, ARG_ENCODING } state = ARG_NONE;
struct ethtool_fecparam feccmd;
- struct cmdline_info cmdline_fec[] = {
- { "encoding", CMDL_STR, &fecmode_str, &feccmd.fec},
- };
- int changed;
- int fecmode;
- int rv;
+ int fecmode = 0, newmode;
+ int rv, i;
- parse_generic_cmdline(ctx, &changed, cmdline_fec,
- ARRAY_SIZE(cmdline_fec));
-
- if (!fecmode_str)
+ for (i = 0; i < ctx->argc; i++) {
+ if (!strcmp(ctx->argp[i], "encoding")) {
+ state = ARG_ENCODING;
+ continue;
+ }
+ if (state == ARG_ENCODING) {
+ newmode = fecmode_str_to_type(ctx->argp[i]);
+ if (!newmode)
+ exit_bad_args();
+ fecmode |= newmode;
+ continue;
+ }
exit_bad_args();
+ }
- fecmode = parse_fecmode(fecmode_str);
if (!fecmode)
exit_bad_args();
@@ -5265,7 +5236,7 @@ static const struct option {
" [ all ]\n"},
{ "--show-fec", 1, do_gfec, "Show FEC settings"},
{ "--set-fec", 1, do_sfec, "Set FEC settings",
- " [ encoding auto|off|rs|baser ]\n"},
+ " [ encoding auto|off|rs|baser [...]]\n"},
{ "-h|--help", 0, show_usage, "Show this help" },
{ "--version", 0, do_version, "Show version number" },
{}
diff --git a/test-cmdline.c b/test-cmdline.c
index 9c51cca..84630a5 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -268,12 +268,12 @@ static struct test_case {
{ 1, "--set-eee devname advertise foo" },
{ 1, "--set-fec devname" },
{ 0, "--set-fec devname encoding auto" },
- { 0, "--set-fec devname encoding off," },
- { 0, "--set-fec devname encoding baser,rs" },
- { 0, "--set-fec devname encoding auto,auto," },
+ { 0, "--set-fec devname encoding off" },
+ { 0, "--set-fec devname encoding baser rs" },
+ { 0, "--set-fec devname encoding auto auto" },
{ 1, "--set-fec devname encoding foo" },
- { 1, "--set-fec devname encoding auto,foo" },
- { 1, "--set-fec devname encoding auto,," },
+ { 1, "--set-fec devname encoding auto foo" },
+ { 1, "--set-fec devname encoding none" },
{ 1, "--set-fec devname auto" },
/* can't test --set-priv-flags yet */
{ 0, "-h" },
^ permalink raw reply related
* [PATCH] net: macb: Clean 64b dma addresses if they are not detected
From: Michal Simek @ 2018-09-19 16:08 UTC (permalink / raw)
To: linux-kernel, monstr, Edgar E. Iglesias
Cc: David S. Miller, netdev, u-boot, Joe Hershberger, Nicolas Ferre
Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
is not detected on 64bit system.
The issue was observed when bootloader(u-boot) does not check macb
feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
by default. Then macb driver is reading DMACFG register back and only
adding 64bit dma configuration but not cleaning it out.
This is also align with other features which are also cleared if they are not
present.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---
drivers/net/ethernet/cadence/macb_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 16e4ef7d7185..79707dff3f13 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
if (bp->hw_dma_cap & HW_DMA_CAP_64B)
dmacfg |= GEM_BIT(ADDR64);
+ else
+ dmacfg &= ~GEM_BIT(ADDR64);
#endif
#ifdef CONFIG_MACB_USE_HWSTAMP
if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 3/7] netlink: set extack error message in nla_validate()
From: David Ahern @ 2018-09-19 16:20 UTC (permalink / raw)
To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-4-johannes@sipsolutions.net>
On 9/19/18 5:08 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> In nla_parse() we already set this, but it makes sense to
> also do it in nla_validate() which already also sets the
> bad attribute pointer.
>
> CC: David Ahern <dsahern@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> lib/nlattr.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index e2e5b38394d5..33404745bec4 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -180,9 +180,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
> int rem;
>
> nla_for_each_attr(nla, head, len, rem) {
> - int err = validate_nla(nla, maxtype, policy, NULL);
> + static const char _msg[] = "Attribute failed policy validation";
> + const char *msg = _msg;
> + int err = validate_nla(nla, maxtype, policy, &msg);
>
> if (err < 0) {
> + if (extack)
> + extack->_msg = msg;
> NL_SET_BAD_ATTR(extack, nla);
msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla
removing the need for the same message ("Attribute failed policy
validation") to be declared twice and simplifying the extack setting.
> return err;
> }
>
^ permalink raw reply
* Re: [PATCH 2/7] netlink: make validation_data const
From: David Ahern @ 2018-09-19 16:21 UTC (permalink / raw)
To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-3-johannes@sipsolutions.net>
On 9/19/18 5:08 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> The validation data is only used within the policy that
> should usually already be const, and isn't changed in any
> code that uses it. Therefore, make the validation_data
> pointer const.
>
> While at it, remove the duplicate variable in the bitfield
> validation that I'd otherwise have to change to const.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> include/net/netlink.h | 2 +-
> lib/nlattr.c | 5 ++---
> 2 files changed, 3 insertions(+), 4 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* [PATCH 03/22] linux/net.h: use DYNAMIC_DEBUG_BRANCH in net_dbg_ratelimited
From: Rasmus Villemoes @ 2018-09-19 22:04 UTC (permalink / raw)
To: Jason Baron, Andrew Morton; +Cc: linux-kernel, Rasmus Villemoes, netdev
In-Reply-To: <20180919220444.23190-1-linux@rasmusvillemoes.dk>
net_dbg_ratelimited tests the dynamic debug descriptor the old-fashioned
way, and doesn't utilize the static key/jump label implementation on
architectures that HAVE_JUMP_LABEL. Use the DYNAMIC_DEBUG_BRANCH which
is defined appropriately.
Cc: netdev@vger.kernel.org
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
include/linux/net.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/net.h b/include/linux/net.h
index e0930678c8bf..651fca72286c 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -263,7 +263,7 @@ do { \
#define net_dbg_ratelimited(fmt, ...) \
do { \
DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \
- if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) && \
+ if (DYNAMIC_DEBUG_BRANCH(descriptor) && \
net_ratelimit()) \
__dynamic_pr_debug(&descriptor, pr_fmt(fmt), \
##__VA_ARGS__); \
--
2.16.4
^ permalink raw reply related
* Re: [PATCH 5/7] netlink: prepare validate extack setting for recursion
From: David Ahern @ 2018-09-19 16:28 UTC (permalink / raw)
To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-6-johannes@sipsolutions.net>
On 9/19/18 5:08 AM, Johannes Berg wrote:
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 966cd3dcf31b..2b015e43b725 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -69,7 +69,7 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
>
> static int validate_nla(const struct nlattr *nla, int maxtype,
> const struct nla_policy *policy,
> - const char **error_msg)
> + struct netlink_ext_ack *extack, bool *extack_set)
extack_set arg is not needed if you handle the "Attribute failed policy
validation" message and NL_SET_BAD_ATTR here as well.
^ permalink raw reply
* Re: [PATCH 3/7] netlink: set extack error message in nla_validate()
From: Johannes Berg @ 2018-09-19 16:31 UTC (permalink / raw)
To: David Ahern, linux-wireless, netdev
In-Reply-To: <3e7aadff-5228-660a-537a-9b54c11fa5cc@gmail.com>
On Wed, 2018-09-19 at 09:20 -0700, David Ahern wrote:
> > nla_for_each_attr(nla, head, len, rem) {
> > - int err = validate_nla(nla, maxtype, policy, NULL);
> > + static const char _msg[] = "Attribute failed policy validation";
> > + const char *msg = _msg;
> > + int err = validate_nla(nla, maxtype, policy, &msg);
> >
> > if (err < 0) {
> > + if (extack)
> > + extack->_msg = msg;
> > NL_SET_BAD_ATTR(extack, nla);
>
> msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla
> removing the need for the same message ("Attribute failed policy
> validation") to be declared twice and simplifying the extack setting.
Yeah, perhaps I should take another look at that. I didn't want to do
that originally as validate_nla() has so many exit points, but perhaps
it's better to put a goto label there.
FWIW, in the next patch I'm getting rid of the duplication again - I
wanted to have the patch that has an effect (this one, setting the
message) more clearly separated.
johannes
^ 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