* [PATCH net-next 3/5] net: Add TLS offload netdev ops
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
In-Reply-To: <1505385988-94522-1-git-send-email-ilyal@mellanox.com>
Add new netdev ops to add and delete tls context
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
include/linux/netdevice.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cdfd9ad..4ea81bab 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -826,6 +826,23 @@ struct xfrmdev_ops {
};
#endif
+#if IS_ENABLED(CONFIG_TLS_DEVICE)
+enum tls_offload_ctx_dir {
+ TLS_OFFLOAD_CTX_DIR_RX,
+ TLS_OFFLOAD_CTX_DIR_TX,
+};
+
+struct tls_crypto_info;
+
+struct tlsdev_ops {
+ int (*tls_dev_add)(struct net_device *netdev, struct sock *sk,
+ enum tls_offload_ctx_dir direction,
+ struct tls_crypto_info *crypto_info);
+ void (*tls_dev_del)(struct net_device *netdev, struct sock *sk,
+ enum tls_offload_ctx_dir direction);
+};
+#endif
+
/*
* This structure defines the management hooks for network devices.
* The following hooks can be defined; unless noted otherwise, they are
@@ -1713,6 +1730,10 @@ struct net_device {
const struct xfrmdev_ops *xfrmdev_ops;
#endif
+#if IS_ENABLED(CONFIG_TLS_DEVICE)
+ const struct tlsdev_ops *tlsdev_ops;
+#endif
+
const struct header_ops *header_ops;
unsigned int flags;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 1/5] tls: Move release of tls_ctx into tls_sw_free_resources
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
In-Reply-To: <1505385988-94522-1-git-send-email-ilyal@mellanox.com>
Move release of tls_ctx into sw specific code.
This is required because the device offload implementation
requires this context to remain alive until there are
no more in-flight SKBs.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
net/tls/tls_main.c | 5 ++---
net/tls/tls_sw.c | 1 +
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index 60aff60..ae20ee3 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -232,12 +232,11 @@ static void tls_sk_proto_close(struct sock *sk, long timeout)
sg++;
}
}
- ctx->free_resources(sk);
+
kfree(ctx->rec_seq);
kfree(ctx->iv);
-
sk_proto_close = ctx->sk_proto_close;
- kfree(ctx);
+ ctx->free_resources(sk);
release_sock(sk);
sk_proto_close(sk, timeout);
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fa596fa..db1e566 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -650,6 +650,7 @@ void tls_sw_free_resources(struct sock *sk)
tls_free_both_sg(sk);
kfree(ctx);
+ kfree(tls_ctx);
}
int tls_set_sw_offload(struct sock *sk, struct tls_context *ctx)
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/5] tls: Add generic NIC offload infrastructure
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
This series add a generic infrastructure to offload TLS crypto to a
network devices. It enables the kernel TLS socket to skip encryption and
authentication operations on the transmit side of the data path. Leaving
those computationally expensive operations to the NIC.
The NIC offload infrastructure builds TLS records and pushes them to the
TCP layer just like the SW KTLS implementation and using the same API.
TCP segmentation is mostly unaffected. Currently the only exception is
that we prevent mixed SKBs where only part of the payload requires
offload. In the future we are likely to add a similar restriction
following a change cipher spec record.
The notable differences between SW KTLS and NIC offloaded TLS
implementations are as follows:
1. The offloaded implementation builds "plaintext TLS record", those
records contain plaintext instead of ciphertext and place holder bytes
instead of authentication tags.
2. The offloaded implementation maintains a mapping from TCP sequence
number to TLS records. Thus given a TCP SKB sent from a NIC offloaded
TLS socket, we can use the tls NIC offload infrastructure to obtain
enough context to encrypt the payload of the SKB.
A TLS record is released when the last byte of the record is ack'ed,
this is done through the new icsk_clean_acked callback.
The infrastructure should be extendable to support various NIC offload
implementations. However it is currently written with the
implementation below in mind:
The NIC assumes that packets from each offloaded stream are sent as
plaintext and in-order. It keeps track of the TLS records in the TCP
stream. When a packet marked for offload is transmitted, the NIC
encrypts the payload in-place and puts authentication tags in the
relevant place holders.
The responsibility for handling out-of-order packets (i.e. TCP
retransmission, qdisc drops) falls on the netdev driver.
The netdev driver keeps track of the expected TCP SN from the NIC's
perspective. If the next packet to transmit matches the expected TCP
SN, the driver advances the expected TCP SN, and transmits the packet
with TLS offload indication.
If the next packet to transmit does not match the expected TCP SN. The
driver calls the TLS layer to obtain the TLS record that includes the
TCP of the packet for transmission. Using this TLS record, the driver
posts a work entry on the transmit queue to reconstruct the NIC TLS
state required for the offload of the out-of-order packet. It updates
the expected TCP SN accordingly and transmit the now in-order packet.
The same queue is used for packet transmission and TLS context
reconstruction to avoid the need for flushing the transmit queue before
issuing the context reconstruction request.
Expected TCP SN is accessed without a lock, under the assumption that
TCP doesn't transmit SKBs from different TX queue concurrently.
We assume that packets are not rerouted to a different network device.
Github with mlx5e TLS offload support:
https://github.com/Mellanox/tls-offload/tree/tls_device_v1
Paper: https://www.netdevconf.org/1.2/papers/netdevconf-TLS.pdf
Ilya Lesokhin (5):
tls: Move release of tls_ctx into tls_sw_free_resources
tcp: Add clean acked data hook
net: Add TLS offload netdev ops
net: Add TLS TX offload features
tls: Add generic NIC offload infrastructure.
include/linux/netdev_features.h | 2 +
include/linux/netdevice.h | 21 ++
include/net/inet_connection_sock.h | 2 +
include/net/tls.h | 41 ++-
net/core/ethtool.c | 1 +
net/ipv4/tcp_input.c | 3 +
net/tls/Kconfig | 9 +
net/tls/Makefile | 3 +
net/tls/tls_device.c | 673 +++++++++++++++++++++++++++++++++++++
net/tls/tls_main.c | 68 ++--
net/tls/tls_sw.c | 1 +
11 files changed, 803 insertions(+), 21 deletions(-)
create mode 100644 net/tls/tls_device.c
--
1.8.3.1
^ permalink raw reply
* [PATCH net-next 5/5] tls: Add generic NIC offload infrastructure.
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
In-Reply-To: <1505385988-94522-1-git-send-email-ilyal@mellanox.com>
This patch adds a generic infrastructure to offload TLS crypto to a
network devices. It enables the kernel TLS socket to skip encryption and
authentication operations on the transmit side of the data path. Leaving
those computationally expensive operations to the NIC.
The NIC offload infrastructure builds TLS records and pushes them to the
TCP layer just like the SW KTLS implementation and using the same API.
TCP segmentation is mostly unaffected. Currently the only exception is
that we prevent mixed SKBs where only part of the payload requires
offload. In the future we are likely to add a similar restriction
following a change cipher spec record.
The notable differences between SW KTLS and NIC offloaded TLS
implementations are as follows:
1. The offloaded implementation builds "plaintext TLS record", those
records contain plaintext instead of ciphertext and place holder bytes
instead of authentication tags.
2. The offloaded implementation maintains a mapping from TCP sequence
number to TLS records. Thus given a TCP SKB sent from a NIC offloaded
TLS socket, we can use the tls NIC offload infrastructure to obtain
enough context to encrypt the payload of the SKB.
A TLS record is released when the last byte of the record is ack'ed,
this is done through the new icsk_clean_acked callback.
The infrastructure should be extendable to support various NIC offload
implementations. However it is currently written with the
implementation below in mind:
The NIC assumes that packets from each offloaded stream are sent as
plaintext and in-order. It keeps track of the TLS records in the TCP
stream. When a packet marked for offload is transmitted, the NIC
encrypts the payload in-place and puts authentication tags in the
relevant place holders.
The responsibility for handling out-of-order packets (i.e. TCP
retransmission, qdisc drops) falls on the netdev driver.
The netdev driver keeps track of the expected TCP SN from the NIC's
perspective. If the next packet to transmit matches the expected TCP
SN, the driver advances the expected TCP SN, and transmits the packet
with TLS offload indication.
If the next packet to transmit does not match the expected TCP SN. The
driver calls the TLS layer to obtain the TLS record that includes the
TCP of the packet for transmission. Using this TLS record, the driver
posts a work entry on the transmit queue to reconstruct the NIC TLS
state required for the offload of the out-of-order packet. It updates
the expected TCP SN accordingly and transmit the now in-order packet.
The same queue is used for packet transmission and TLS context
reconstruction to avoid the need for flushing the transmit queue before
issuing the context reconstruction request.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
include/net/tls.h | 41 +++-
net/tls/Kconfig | 9 +
net/tls/Makefile | 3 +
net/tls/tls_device.c | 673 +++++++++++++++++++++++++++++++++++++++++++++++++++
net/tls/tls_main.c | 63 +++--
5 files changed, 771 insertions(+), 18 deletions(-)
create mode 100644 net/tls/tls_device.c
diff --git a/include/net/tls.h b/include/net/tls.h
index b89d397..1f83c8e 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -71,6 +71,24 @@ struct tls_sw_context {
struct scatterlist sg_aead_out[2];
};
+struct tls_record_info {
+ struct list_head list;
+ u32 end_seq;
+ int len;
+ int num_frags;
+ skb_frag_t frags[MAX_SKB_FRAGS];
+};
+
+struct tls_offload_context {
+ struct list_head records_list;
+ struct scatterlist sg_tx_data[MAX_SKB_FRAGS];
+ void (*sk_destruct)(struct sock *sk);
+ struct tls_record_info *open_record;
+ struct tls_record_info *retransmit_hint;
+ u32 expected_seq;
+ spinlock_t lock; /* protects records list */
+};
+
enum {
TLS_PENDING_CLOSED_RECORD
};
@@ -81,6 +99,9 @@ struct tls_context {
struct tls12_crypto_info_aes_gcm_128 crypto_send_aes_gcm_128;
};
+ struct list_head gclist;
+ struct sock *sk;
+ struct net_device *netdev;
void *priv_ctx;
u16 prepend_size;
@@ -123,9 +144,18 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
void tls_sw_close(struct sock *sk, long timeout);
-void tls_sk_destruct(struct sock *sk, struct tls_context *ctx);
-void tls_icsk_clean_acked(struct sock *sk);
+void tls_clear_device_offload(struct sock *sk, struct tls_context *ctx);
+int tls_set_device_offload(struct sock *sk, struct tls_context *ctx);
+int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
+int tls_device_sendpage(struct sock *sk, struct page *page,
+ int offset, size_t size, int flags);
+void tls_device_sk_destruct(struct sock *sk);
+void tls_device_cleanup(void);
+struct tls_record_info *tls_get_record(struct tls_offload_context *context,
+ u32 seq);
+
+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);
@@ -162,6 +192,13 @@ static inline bool tls_is_pending_open_record(struct tls_context *tls_ctx)
return tls_ctx->pending_open_record_frags;
}
+static inline bool tls_is_sk_tx_device_offloaded(struct sock *sk)
+{
+ /* matches smp_store_release in tls_set_device_offload */
+ return smp_load_acquire(&sk->sk_destruct) ==
+ &tls_device_sk_destruct;
+}
+
static inline void tls_err_abort(struct sock *sk)
{
sk->sk_err = -EBADMSG;
diff --git a/net/tls/Kconfig b/net/tls/Kconfig
index eb58303..1a4ea55c 100644
--- a/net/tls/Kconfig
+++ b/net/tls/Kconfig
@@ -13,3 +13,12 @@ config TLS
encryption handling of the TLS protocol to be done in-kernel.
If unsure, say N.
+
+config TLS_DEVICE
+ bool "Transport Layer Security HW offload"
+ depends on TLS
+ default n
+ ---help---
+ Enable kernel support for HW offload of the TLS protocol.
+
+ If unsure, say N.
diff --git a/net/tls/Makefile b/net/tls/Makefile
index a930fd1..9de5055 100644
--- a/net/tls/Makefile
+++ b/net/tls/Makefile
@@ -5,3 +5,6 @@
obj-$(CONFIG_TLS) += tls.o
tls-y := tls_main.o tls_sw.o
+
+tls-$(CONFIG_TLS_DEVICE) += tls_device.o
+
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
new file mode 100644
index 0000000..94a25c2
--- /dev/null
+++ b/net/tls/tls_device.c
@@ -0,0 +1,673 @@
+/* Copyright (c) 2016-2017, Mellanox Technologies All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials
+ * provided with the distribution.
+ *
+ * - Neither the name of the Mellanox Technologies nor the
+ * names of its contributors may be used to endorse or promote
+ * products derived from this software without specific prior written
+ * permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED.
+ * IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
+ * IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE
+ */
+
+#include <linux/module.h>
+#include <net/tcp.h>
+#include <net/inet_common.h>
+#include <linux/highmem.h>
+#include <linux/netdevice.h>
+
+#include <net/tls.h>
+
+static void tls_device_gc_task(struct work_struct *work);
+
+static DECLARE_WORK(tls_device_gc_work, tls_device_gc_task);
+static LIST_HEAD(tls_device_gc_list);
+static DEFINE_SPINLOCK(tls_device_gc_lock);
+
+static void tls_device_gc_task(struct work_struct *work)
+{
+ struct tls_context *ctx, *tmp;
+ struct list_head gc_list;
+ unsigned long flags;
+
+ spin_lock_irqsave(&tls_device_gc_lock, flags);
+ INIT_LIST_HEAD(&gc_list);
+ list_splice_init(&tls_device_gc_list, &gc_list);
+ spin_unlock_irqrestore(&tls_device_gc_lock, flags);
+
+ list_for_each_entry_safe(ctx, tmp, &gc_list, gclist) {
+ struct tls_offload_context *offlad_ctx = tls_offload_ctx(ctx);
+ void (*sk_destruct)(struct sock *sk) = offlad_ctx->sk_destruct;
+ struct net_device *netdev = ctx->netdev;
+ struct sock *sk = ctx->sk;
+
+ netdev->tlsdev_ops->tls_dev_del(netdev, sk,
+ TLS_OFFLOAD_CTX_DIR_TX);
+
+ list_del(&ctx->gclist);
+ kfree(offlad_ctx);
+ kfree(ctx);
+ sk_destruct(sk);
+ }
+}
+
+static void tls_device_queue_ctx_destruction(struct tls_context *ctx)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&tls_device_gc_lock, flags);
+ list_add_tail(&ctx->gclist, &tls_device_gc_list);
+ spin_unlock_irqrestore(&tls_device_gc_lock, flags);
+
+ schedule_work(&tls_device_gc_work);
+}
+
+/* We assume that the socket is already connected */
+static struct net_device *get_netdev_for_sock(struct sock *sk)
+{
+ struct inet_sock *inet = inet_sk(sk);
+ struct net_device *netdev = NULL;
+
+ netdev = dev_get_by_index(sock_net(sk), inet->cork.fl.flowi_oif);
+
+ return netdev;
+}
+
+static void detach_sock_from_netdev(struct sock *sk, struct tls_context *ctx)
+{
+ struct net_device *netdev;
+
+ netdev = get_netdev_for_sock(sk);
+ if (!netdev) {
+ pr_err("got offloaded socket with no netdev\n");
+ return;
+ }
+
+ if (!netdev->tlsdev_ops) {
+ pr_err("attach_sock_to_netdev: netdev %s with no TLS offload\n",
+ netdev->name);
+ return;
+ }
+
+ netdev->tlsdev_ops->tls_dev_del(netdev, sk, TLS_OFFLOAD_CTX_DIR_TX);
+ dev_put(netdev);
+}
+
+static int attach_sock_to_netdev(struct sock *sk, struct net_device *netdev,
+ struct tls_context *ctx)
+{
+ int rc;
+
+ rc = netdev->tlsdev_ops->tls_dev_add(
+ netdev,
+ sk,
+ TLS_OFFLOAD_CTX_DIR_TX,
+ &ctx->crypto_send);
+ if (rc) {
+ pr_err("The netdev has refused to offload this socket\n");
+ goto out;
+ }
+
+ sk->sk_bound_dev_if = netdev->ifindex;
+ sk_dst_reset(sk);
+
+ rc = 0;
+out:
+ return rc;
+}
+
+static void destroy_record(struct tls_record_info *record)
+{
+ skb_frag_t *frag;
+ int nr_frags = record->num_frags;
+
+ while (nr_frags > 0) {
+ frag = &record->frags[nr_frags - 1];
+ __skb_frag_unref(frag);
+ --nr_frags;
+ }
+ kfree(record);
+}
+
+static void delete_all_records(struct tls_offload_context *offload_ctx)
+{
+ struct tls_record_info *info, *temp;
+
+ list_for_each_entry_safe(info, temp, &offload_ctx->records_list, list) {
+ list_del(&info->list);
+ destroy_record(info);
+ }
+
+ offload_ctx->retransmit_hint = NULL;
+}
+
+static void tls_icsk_clean_acked(struct sock *sk)
+{
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_offload_context *ctx;
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct tls_record_info *info, *temp;
+ unsigned long flags;
+
+ if (!tls_ctx)
+ return;
+
+ ctx = tls_offload_ctx(tls_ctx);
+
+ spin_lock_irqsave(&ctx->lock, flags);
+ info = ctx->retransmit_hint;
+ if (info && !before(tp->snd_una, info->end_seq)) {
+ ctx->retransmit_hint = NULL;
+ list_del(&info->list);
+ destroy_record(info);
+ }
+
+ list_for_each_entry_safe(info, temp, &ctx->records_list, list) {
+ if (before(tp->snd_una, info->end_seq))
+ break;
+ list_del(&info->list);
+
+ destroy_record(info);
+ }
+
+ spin_unlock_irqrestore(&ctx->lock, flags);
+}
+
+static void tls_device_free_resources(struct sock *sk)
+{
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
+
+ if (ctx->open_record)
+ destroy_record(ctx->open_record);
+}
+
+/* At this point, there should be no references on this
+ * socket and no in-flight SKBs associated with this
+ * socket, so it is safe to free all the resources.
+ */
+void tls_device_sk_destruct(struct sock *sk)
+{
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
+
+ delete_all_records(ctx);
+
+ tls_device_queue_ctx_destruction(tls_ctx);
+}
+EXPORT_SYMBOL(tls_device_sk_destruct);
+
+static inline void tls_append_frag(struct tls_record_info *record,
+ struct page_frag *pfrag,
+ int size)
+{
+ skb_frag_t *frag;
+
+ frag = &record->frags[record->num_frags - 1];
+ if (frag->page.p == pfrag->page &&
+ frag->page_offset + frag->size == pfrag->offset) {
+ frag->size += size;
+ } else {
+ ++frag;
+ frag->page.p = pfrag->page;
+ frag->page_offset = pfrag->offset;
+ frag->size = size;
+ ++record->num_frags;
+ get_page(pfrag->page);
+ }
+
+ pfrag->offset += size;
+ record->len += size;
+}
+
+static inline int tls_push_record(struct sock *sk,
+ struct tls_context *ctx,
+ struct tls_offload_context *offload_ctx,
+ struct tls_record_info *record,
+ struct page_frag *pfrag,
+ int flags,
+ unsigned char record_type)
+{
+ skb_frag_t *frag;
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct page_frag fallback_frag;
+ struct page_frag *tag_pfrag = pfrag;
+ int i;
+
+ /* fill prepand */
+ frag = &record->frags[0];
+ tls_fill_prepend(ctx,
+ skb_frag_address(frag),
+ record->len - ctx->prepend_size,
+ record_type);
+
+ if (unlikely(!skb_page_frag_refill(
+ ctx->tag_size,
+ pfrag, GFP_KERNEL))) {
+ /* HW doesn't care about the data in the tag
+ * so in case pfrag has no room
+ * for a tag and we can't allocate a new pfrag
+ * just use the page in the first frag
+ * rather then write a complicated fall back code.
+ */
+ tag_pfrag = &fallback_frag;
+ tag_pfrag->page = skb_frag_page(frag);
+ tag_pfrag->offset = 0;
+ }
+
+ tls_append_frag(record, tag_pfrag, ctx->tag_size);
+ record->end_seq = tp->write_seq + record->len;
+ spin_lock_irq(&offload_ctx->lock);
+ list_add_tail(&record->list, &offload_ctx->records_list);
+ spin_unlock_irq(&offload_ctx->lock);
+ offload_ctx->open_record = NULL;
+ set_bit(TLS_PENDING_CLOSED_RECORD, &ctx->flags);
+ tls_advance_record_sn(sk, ctx);
+
+ for (i = 0; i < record->num_frags; i++) {
+ frag = &record->frags[i];
+ sg_unmark_end(&offload_ctx->sg_tx_data[i]);
+ sg_set_page(&offload_ctx->sg_tx_data[i], skb_frag_page(frag),
+ frag->size, frag->page_offset);
+ sk_mem_charge(sk, frag->size);
+ get_page(skb_frag_page(frag));
+ }
+ sg_mark_end(&offload_ctx->sg_tx_data[record->num_frags - 1]);
+
+ /* all ready, send */
+ return tls_push_sg(sk, ctx, offload_ctx->sg_tx_data, 0, flags);
+}
+
+static inline int tls_create_new_record(
+ struct tls_offload_context *offload_ctx,
+ struct page_frag *pfrag,
+ size_t prepend_size)
+{
+ skb_frag_t *frag;
+ struct tls_record_info *record;
+
+ record = kmalloc(sizeof(*record), GFP_KERNEL);
+ if (!record)
+ return -ENOMEM;
+
+ frag = &record->frags[0];
+ __skb_frag_set_page(frag, pfrag->page);
+ frag->page_offset = pfrag->offset;
+ skb_frag_size_set(frag, prepend_size);
+
+ get_page(pfrag->page);
+ pfrag->offset += prepend_size;
+
+ record->num_frags = 1;
+ record->len = prepend_size;
+ offload_ctx->open_record = record;
+ return 0;
+}
+
+static inline int tls_do_allocation(
+ struct sock *sk,
+ struct tls_offload_context *offload_ctx,
+ struct page_frag *pfrag,
+ size_t prepend_size)
+{
+ int ret;
+
+ if (!offload_ctx->open_record) {
+ if (unlikely(!skb_page_frag_refill(prepend_size, pfrag,
+ sk->sk_allocation))) {
+ sk->sk_prot->enter_memory_pressure(sk);
+ sk_stream_moderate_sndbuf(sk);
+ return -ENOMEM;
+ }
+
+ ret = tls_create_new_record(offload_ctx, pfrag, prepend_size);
+ if (ret)
+ return ret;
+
+ if (pfrag->size > pfrag->offset)
+ return 0;
+ }
+
+ if (!sk_page_frag_refill(sk, pfrag))
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int tls_push_data(struct sock *sk,
+ struct iov_iter *msg_iter,
+ size_t size, int flags,
+ unsigned char record_type)
+{
+ struct tls_context *tls_ctx = tls_get_ctx(sk);
+ struct tls_offload_context *ctx = tls_offload_ctx(tls_ctx);
+ struct tls_record_info *record = ctx->open_record;
+ struct page_frag *pfrag;
+ int copy, rc = 0;
+ size_t orig_size = size;
+ u32 max_open_record_len;
+ long timeo;
+ int more = flags & (MSG_SENDPAGE_NOTLAST | MSG_MORE);
+ int tls_push_record_flags = flags | MSG_SENDPAGE_NOTLAST;
+ bool done = false;
+
+ if (sk->sk_err)
+ return -sk->sk_err;
+
+ timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
+ rc = tls_complete_pending_work(sk, tls_ctx, flags, &timeo);
+ if (rc < 0)
+ return rc;
+
+ pfrag = sk_page_frag(sk);
+
+ /* KTLS_TLS_HEADER_SIZE is not counted as part of the TLS record, and
+ * we need to leave room for an authentication tag.
+ */
+ max_open_record_len = TLS_MAX_PAYLOAD_SIZE +
+ tls_ctx->prepend_size;
+ do {
+ if (tls_do_allocation(sk, ctx, pfrag,
+ tls_ctx->prepend_size)) {
+ rc = sk_stream_wait_memory(sk, &timeo);
+ if (!rc)
+ continue;
+
+ record = ctx->open_record;
+ if (!record)
+ break;
+handle_error:
+ if (record_type != TLS_RECORD_TYPE_DATA) {
+ /* avoid sending partial
+ * record with type !=
+ * application_data
+ */
+ size = orig_size;
+ destroy_record(record);
+ ctx->open_record = NULL;
+ } else if (record->len > tls_ctx->prepend_size) {
+ goto last_record;
+ }
+
+ break;
+ }
+
+ record = ctx->open_record;
+ copy = min_t(size_t, size, (pfrag->size - pfrag->offset));
+ copy = min_t(size_t, copy, (max_open_record_len - record->len));
+
+ if (copy_from_iter_nocache(
+ page_address(pfrag->page) + pfrag->offset,
+ copy, msg_iter) != copy) {
+ rc = -EFAULT;
+ goto handle_error;
+ }
+ tls_append_frag(record, pfrag, copy);
+
+ size -= copy;
+ if (!size) {
+last_record:
+ tls_push_record_flags = flags;
+ if (more) {
+ tls_ctx->pending_open_record_frags =
+ record->num_frags;
+ break;
+ }
+
+ done = true;
+ }
+
+ if ((done) ||
+ (record->len >= max_open_record_len) ||
+ (record->num_frags >= MAX_SKB_FRAGS - 1)) {
+ rc = tls_push_record(sk,
+ tls_ctx,
+ ctx,
+ record,
+ pfrag,
+ tls_push_record_flags,
+ record_type);
+ if (rc < 0)
+ break;
+ }
+ } while (!done);
+
+ if (orig_size - size > 0)
+ rc = orig_size - size;
+
+ return rc;
+}
+
+int tls_device_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
+{
+ unsigned char record_type = TLS_RECORD_TYPE_DATA;
+ int rc = 0;
+
+ lock_sock(sk);
+
+ if (unlikely(msg->msg_controllen)) {
+ rc = tls_proccess_cmsg(sk, msg, &record_type);
+ if (rc)
+ goto out;
+ }
+
+ rc = tls_push_data(sk, &msg->msg_iter, size,
+ msg->msg_flags, record_type);
+
+out:
+ release_sock(sk);
+ return rc;
+}
+
+int tls_device_sendpage(struct sock *sk, struct page *page,
+ int offset, size_t size, int flags)
+{
+ struct iov_iter msg_iter;
+ struct kvec iov;
+ char *kaddr = kmap(page);
+ int rc = 0;
+
+ if (flags & MSG_SENDPAGE_NOTLAST)
+ flags |= MSG_MORE;
+
+ lock_sock(sk);
+
+ if (flags & MSG_OOB) {
+ rc = -ENOTSUPP;
+ goto out;
+ }
+
+ iov.iov_base = kaddr + offset;
+ iov.iov_len = size;
+ iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, &iov, 1, size);
+ rc = tls_push_data(sk, &msg_iter, size,
+ flags, TLS_RECORD_TYPE_DATA);
+ kunmap(page);
+
+out:
+ release_sock(sk);
+ return rc;
+}
+
+struct tls_record_info *tls_get_record(struct tls_offload_context *context,
+ u32 seq)
+{
+ struct tls_record_info *info;
+
+ info = context->retransmit_hint;
+ if (!info ||
+ before(seq, info->end_seq - info->len))
+ info = list_first_entry(&context->records_list,
+ struct tls_record_info, list);
+
+ list_for_each_entry_from(info, &context->records_list, list) {
+ if (before(seq, info->end_seq)) {
+ if (!context->retransmit_hint ||
+ after(info->end_seq,
+ context->retransmit_hint->end_seq))
+ context->retransmit_hint = info;
+ return info;
+ }
+ }
+
+ return NULL;
+}
+EXPORT_SYMBOL(tls_get_record);
+
+static int tls_device_push_pending_record(struct sock *sk, int flags)
+{
+ struct iov_iter msg_iter;
+
+ iov_iter_kvec(&msg_iter, WRITE | ITER_KVEC, NULL, 0, 0);
+ return tls_push_data(sk, &msg_iter, 0, flags, TLS_RECORD_TYPE_DATA);
+}
+
+int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
+{
+ struct tls_crypto_info *crypto_info;
+ struct tls_offload_context *offload_ctx;
+ struct tls_record_info *start_marker_record;
+ u16 nonece_size, tag_size, iv_size, rec_seq_size;
+ char *iv, *rec_seq;
+ int rc;
+ struct net_device *netdev;
+ struct sk_buff *skb;
+
+ if (!ctx) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (ctx->priv_ctx) {
+ rc = -EEXIST;
+ goto out;
+ }
+
+ netdev = get_netdev_for_sock(sk);
+ if (!netdev) {
+ pr_err("%s: netdev not found\n", __func__);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (!(netdev->features & NETIF_F_HW_TLS_TX)) {
+ rc = -ENOTSUPP;
+ goto release_netdev;
+ }
+
+ crypto_info = &ctx->crypto_send;
+ switch (crypto_info->cipher_type) {
+ case TLS_CIPHER_AES_GCM_128: {
+ nonece_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
+ tag_size = TLS_CIPHER_AES_GCM_128_TAG_SIZE;
+ iv_size = TLS_CIPHER_AES_GCM_128_IV_SIZE;
+ iv = ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->iv;
+ rec_seq_size = TLS_CIPHER_AES_GCM_128_REC_SEQ_SIZE;
+ rec_seq =
+ ((struct tls12_crypto_info_aes_gcm_128 *)crypto_info)->rec_seq;
+ break;
+ }
+ default:
+ rc = -EINVAL;
+ goto release_netdev;
+ }
+
+ start_marker_record = kmalloc(sizeof(*start_marker_record), GFP_KERNEL);
+ if (!start_marker_record) {
+ rc = -ENOMEM;
+ goto release_netdev;
+ }
+
+ rc = attach_sock_to_netdev(sk, netdev, ctx);
+ if (rc)
+ goto free_marker_record;
+
+ ctx->netdev = netdev;
+ ctx->sk = sk;
+
+ ctx->prepend_size = TLS_HEADER_SIZE + nonece_size;
+ ctx->tag_size = tag_size;
+ ctx->iv_size = iv_size;
+ ctx->iv = kmalloc(iv_size + TLS_CIPHER_AES_GCM_128_SALT_SIZE,
+ GFP_KERNEL);
+ if (!ctx->iv) {
+ rc = -ENOMEM;
+ goto detach_sock;
+ }
+ memcpy(ctx->iv + TLS_CIPHER_AES_GCM_128_SALT_SIZE, iv, iv_size);
+ ctx->rec_seq_size = rec_seq_size;
+ ctx->rec_seq = kmalloc(rec_seq_size, GFP_KERNEL);
+ if (!ctx->rec_seq) {
+ rc = -ENOMEM;
+ goto err_iv;
+ }
+ memcpy(ctx->rec_seq, rec_seq, rec_seq_size);
+
+ offload_ctx = ctx->priv_ctx;
+ start_marker_record->end_seq = tcp_sk(sk)->write_seq;
+ start_marker_record->len = 0;
+ start_marker_record->num_frags = 0;
+
+ INIT_LIST_HEAD(&offload_ctx->records_list);
+ list_add_tail(&start_marker_record->list, &offload_ctx->records_list);
+ spin_lock_init(&offload_ctx->lock);
+
+ inet_csk(sk)->icsk_clean_acked = &tls_icsk_clean_acked;
+ ctx->push_pending_record = tls_device_push_pending_record;
+ ctx->free_resources = tls_device_free_resources;
+ offload_ctx->sk_destruct = sk->sk_destruct;
+
+ /* TLS offload is greatly simplified if we don't send
+ * SKBs where only part of the payload needs to be encrypted.
+ * So mark the last skb in the write queue as end of record.
+ */
+ skb = tcp_write_queue_tail(sk);
+ if (skb)
+ TCP_SKB_CB(skb)->eor = 1;
+
+ /* After the next line tls_is_sk_tx_device_offloaded
+ * will return true and ndo_start_xmit might access the
+ * offload context
+ */
+ smp_store_release(&sk->sk_destruct,
+ &tls_device_sk_destruct);
+ goto release_netdev;
+
+err_iv:
+ kfree(ctx->iv);
+detach_sock:
+ detach_sock_from_netdev(sk, ctx);
+free_marker_record:
+ kfree(start_marker_record);
+release_netdev:
+ dev_put(netdev);
+out:
+ return rc;
+}
+
+void __exit tls_device_cleanup(void)
+{
+ flush_work(&tls_device_gc_work);
+}
diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c
index ae20ee3..a93a712 100644
--- a/net/tls/tls_main.c
+++ b/net/tls/tls_main.c
@@ -45,8 +45,16 @@
MODULE_DESCRIPTION("Transport Layer Security Support");
MODULE_LICENSE("Dual BSD/GPL");
-static struct proto tls_base_prot;
-static struct proto tls_sw_prot;
+enum {
+ TLS_BASE_TX,
+ TLS_SW_TX,
+#ifdef CONFIG_TLS_DEVICE
+ TLS_HW_TX,
+#endif
+ TLS_NUM_CONFIG,
+};
+
+static struct proto tls_prots[TLS_NUM_CONFIG];
int wait_on_pending_writer(struct sock *sk, long *timeo)
{
@@ -393,11 +401,19 @@ static int do_tls_setsockopt_tx(struct sock *sk, char __user *optval,
ctx->sk_proto_close = sk->sk_prot->close;
- /* currently SW is default, we will have ethtool in future */
- rc = tls_set_sw_offload(sk, ctx);
- prot = &tls_sw_prot;
- if (rc)
- goto err_crypto_info;
+#ifdef CONFIG_TLS_DEVICE
+ rc = tls_set_device_offload(sk, ctx);
+ prot = &tls_prots[TLS_HW_TX];
+ if (rc) {
+#else
+ {
+#endif
+ /* if HW offload fails fallback to SW */
+ rc = tls_set_sw_offload(sk, ctx);
+ prot = &tls_prots[TLS_SW_TX];
+ if (rc)
+ goto err_crypto_info;
+ }
sk->sk_prot = prot;
goto out;
@@ -452,7 +468,8 @@ static int tls_init(struct sock *sk)
icsk->icsk_ulp_data = ctx;
ctx->setsockopt = sk->sk_prot->setsockopt;
ctx->getsockopt = sk->sk_prot->getsockopt;
- sk->sk_prot = &tls_base_prot;
+
+ sk->sk_prot = &tls_prots[TLS_BASE_TX];
out:
return rc;
}
@@ -463,16 +480,27 @@ static int tls_init(struct sock *sk)
.init = tls_init,
};
-static int __init tls_register(void)
+static void build_protos(struct proto *prot, struct proto *base)
{
- tls_base_prot = tcp_prot;
- tls_base_prot.setsockopt = tls_setsockopt;
- tls_base_prot.getsockopt = tls_getsockopt;
+ prot[TLS_BASE_TX] = *base;
+ prot[TLS_BASE_TX].setsockopt = tls_setsockopt;
+ prot[TLS_BASE_TX].getsockopt = tls_getsockopt;
+
+ prot[TLS_SW_TX] = prot[TLS_BASE_TX];
+ prot[TLS_SW_TX].close = tls_sk_proto_close;
+ prot[TLS_SW_TX].sendmsg = tls_sw_sendmsg;
+ prot[TLS_SW_TX].sendpage = tls_sw_sendpage;
+
+#ifdef CONFIG_TLS_DEVICE
+ prot[TLS_HW_TX] = prot[TLS_SW_TX];
+ prot[TLS_HW_TX].sendmsg = tls_device_sendmsg;
+ prot[TLS_HW_TX].sendpage = tls_device_sendpage;
+#endif
+}
- tls_sw_prot = tls_base_prot;
- tls_sw_prot.sendmsg = tls_sw_sendmsg;
- tls_sw_prot.sendpage = tls_sw_sendpage;
- tls_sw_prot.close = tls_sk_proto_close;
+static int __init tls_register(void)
+{
+ build_protos(tls_prots, &tcp_prot);
tcp_register_ulp(&tcp_tls_ulp_ops);
@@ -482,6 +510,9 @@ static int __init tls_register(void)
static void __exit tls_unregister(void)
{
tcp_unregister_ulp(&tcp_tls_ulp_ops);
+#ifdef CONFIG_TLS_DEVICE
+ tls_device_cleanup();
+#endif
}
module_init(tls_register);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: Add TLS TX offload features
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
In-Reply-To: <1505385988-94522-1-git-send-email-ilyal@mellanox.com>
This patch adds a netdev feature to configure TLS TX offloads.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
include/linux/netdev_features.h | 2 ++
net/core/ethtool.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index dc8b489..ed0648a 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -76,6 +76,7 @@ enum {
NETIF_F_HW_ESP_BIT, /* Hardware ESP transformation offload */
NETIF_F_HW_ESP_TX_CSUM_BIT, /* ESP with TX checksum offload */
NETIF_F_RX_UDP_TUNNEL_PORT_BIT, /* Offload of RX port for UDP tunnels */
+ NETIF_F_HW_TLS_TX_BIT, /* Hardware TLS TX offload */
/*
* Add your fresh new feature above and remember to update
@@ -140,6 +141,7 @@ enum {
#define NETIF_F_HW_ESP __NETIF_F(HW_ESP)
#define NETIF_F_HW_ESP_TX_CSUM __NETIF_F(HW_ESP_TX_CSUM)
#define NETIF_F_RX_UDP_TUNNEL_PORT __NETIF_F(RX_UDP_TUNNEL_PORT)
+#define NETIF_F_HW_TLS_TX __NETIF_F(HW_TLS_TX)
#define for_each_netdev_feature(mask_addr, bit) \
for_each_set_bit(bit, (unsigned long *)mask_addr, NETDEV_FEATURE_COUNT)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6a582ae..2ae1fc4 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -106,6 +106,7 @@ int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info)
[NETIF_F_HW_ESP_BIT] = "esp-hw-offload",
[NETIF_F_HW_ESP_TX_CSUM_BIT] = "esp-tx-csum-hw-offload",
[NETIF_F_RX_UDP_TUNNEL_PORT_BIT] = "rx-udp_tunnel-port-offload",
+ [NETIF_F_HW_TLS_TX_BIT] = "tls-hw-tx-offload",
};
static const char
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 2/5] tcp: Add clean acked data hook
From: Ilya Lesokhin @ 2017-09-14 10:46 UTC (permalink / raw)
To: netdev, davem; +Cc: davejwatson, tom, hannes, borisp, ilyal, aviadye, liranl
In-Reply-To: <1505385988-94522-1-git-send-email-ilyal@mellanox.com>
Called when a TCP segment is acknowledged.
Could be used by application protocols who hold additional
metadata associated with the stream data
This is required by TLS device offload to release
metadata associated with acknowledged TLS records.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Signed-off-by: Ilya Lesokhin <ilyal@mellanox.com>
Signed-off-by: Aviad Yehezkel <aviadye@mellanox.com>
---
include/net/inet_connection_sock.h | 2 ++
net/ipv4/tcp_input.c | 3 +++
2 files changed, 5 insertions(+)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 13e4c89..2ea1026 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -77,6 +77,7 @@ struct inet_connection_sock_af_ops {
* @icsk_af_ops Operations which are AF_INET{4,6} specific
* @icsk_ulp_ops Pluggable ULP control hook
* @icsk_ulp_data ULP private data
+ * @icsk_clean_acked Clean acked data hook
* @icsk_ca_state: Congestion control state
* @icsk_retransmits: Number of unrecovered [RTO] timeouts
* @icsk_pending: Scheduled timer event
@@ -101,6 +102,7 @@ struct inet_connection_sock {
const struct inet_connection_sock_af_ops *icsk_af_ops;
const struct tcp_ulp_ops *icsk_ulp_ops;
void *icsk_ulp_data;
+ void (*icsk_clean_acked)(struct sock *sk);
unsigned int (*icsk_sync_mss)(struct sock *sk, u32 pmtu);
__u8 icsk_ca_state:6,
icsk_ca_setsockopt:1,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index c5d7656..37ebdbb 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3645,6 +3645,9 @@ static int tcp_ack(struct sock *sk, const struct sk_buff *skb, int flag)
if (!prior_packets)
goto no_queue;
+ if (icsk->icsk_clean_acked)
+ icsk->icsk_clean_acked(sk);
+
/* See if we can take anything off of the retransmit queue. */
flag |= tcp_clean_rtx_queue(sk, prior_fackets, prior_snd_una, &acked,
&sack_state);
--
1.8.3.1
^ permalink raw reply related
* System NIC does not get activated at boot time
From: Itaru Kitayama @ 2017-09-14 10:41 UTC (permalink / raw)
To: netdev
Hi,
Our Cavium ThunderX system's system NIC can not get activated upon boot
for some reason. To make it active, we need to re-plug the network
cable. The system is GIGABYTE R150-T62-00 (T45 BIOS), and is powered by
CentOS 7 (kernel is 4.5.0-25.el7). Has anyone seen this kind of issue on
a ThunderX system?
Itaru
^ permalink raw reply
* [PATCH] net/mlx5: fpga: avoid uninitialized return codes
From: Arnd Bergmann @ 2017-09-14 11:06 UTC (permalink / raw)
To: Ilan Tayari, Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: Arnd Bergmann, Boris Pismenny, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
calling mlx5_fpga_mem_{read,write}_i2c() with a zero length on
older compiler version such as gcc-4.6 results in a warning that
the return code is not initialized:
drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:147:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:126:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
On newer compilers, the 'err' variable is optimized away in this
code path and assumed to be zero when the loop completes, so we
don't get this warning.
I'm changing the function here to instead return -EINVAL for the
case, under the assumption that it was never meant to be called
with a zero length argument.
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
index 3c11d6e2160a..914fb9d77a1a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
@@ -64,7 +64,7 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
size_t bytes_done = 0;
u8 actual_size;
- int err;
+ int err = -EINVAL;
if (!fdev->mdev)
return -ENOTCONN;
@@ -93,7 +93,7 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
size_t bytes_done = 0;
u8 actual_size;
- int err;
+ int err = -EINVAL;
if (!fdev->mdev)
return -ENOTCONN;
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-14 11:22 UTC (permalink / raw)
To: nixiaoming
Cc: David Miller, Eric Dumazet, waltje, gw4pts, Andrey Konovalov,
Tobias Klauser, philip.pettersson, Alexander Potapenko,
Network Development, LKML, dede.wu
In-Reply-To: <20170914024046.92505-1-nixiaoming@huawei.com>
On Wed, Sep 13, 2017 at 10:40 PM, nixiaoming <nixiaoming@huawei.com> wrote:
> If fanout_add is preempted after running po-> fanout = match
> and before running __fanout_link,
> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>
> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
> or add spin_lock(&po->bind_lock) before po-> fanout = match
>
> test on linux 4.1.42:
> ./trinity -c setsockopt -C 2 -X &
>
> BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
> Kernel panic - not syncing: BUG!
> CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
> Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
> Call trace:
> [<ffffffc000209414>] dump_backtrace+0x0/0xf8
> [<ffffffc00020952c>] show_stack+0x20/0x28
> [<ffffffc000635574>] dump_stack+0xac/0xe4
> [<ffffffc000633fb8>] panic+0xf8/0x268
> [<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
> [<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
> [<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
> [<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> Tested-by: wudesheng <dede.wu@huawei.com>
> ---
> net/packet/af_packet.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 008a45c..0300146 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -365,10 +365,12 @@ static void __unregister_prot_hook(struct sock *sk, bool sync)
>
> po->running = 0;
>
> + mutex_lock(&fanout_mutex);
> if (po->fanout)
> __fanout_unlink(sk, po);
> else
> __dev_remove_pack(&po->prot_hook);
> + mutex_unlock(&fanout_mutex);
>
> __sock_put(sk);
I happened to be looking at the same or a very similar race, courtesy
of syzkaller. packet_set_ring and fanout_add can race.
I believe that one bug is in fanout_add removing the socket
protocol hook and adding the fanout protocol hook without holding
po->bind_lock.
That lock ensures atomic updates to po->running and the actual
protocol hook. fanout_add tests po->running without holding the lock
if (!po->running)
goto out;
and later unconditionally unbinds the socket protocol hook and binds
the fanout group protocol hook:
if (refcount_read(&match->sk_ref) < PACKET_FANOUT_MAX) {
__dev_remove_pack(&po->prot_hook);
po->fanout = match;
refcount_set(&match->sk_ref,
refcount_read(&match->sk_ref) + 1);
__fanout_link(sk, po);
err = 0;
}
This can happen after packet_set_ring has already removed the
protocol hook, causing the socket to be added to the fanout list
twice.
Testing po->running again, this time while holding the bind_lock,
ensures that packet_set_ring cannot have dropped it in between:
+ spin_lock(&po->bind_lock);
+ if (!po->running) {
+ net_err_ratelimited("fanout add, but
unbound sock");
+ err = -EFAULT;
+ spin_unlock(&po->bind_lock);
+ goto out;
+ }
+ __dev_remove_pack(&po->prot_hook));
po->fanout = match;
refcount_set(&match->sk_ref,
refcount_read(&match->sk_ref) + 1);
__fanout_link(sk, po);
+ spin_unlock(&po->bind_lock);
I verified that the reproducer logs plenty of "fanout add, but unbound
sock" messages.
I intend to send this fix after cleaning it up a bit. Will take a
closer look at your patch to see whether these are indeed the
same bug report.
^ permalink raw reply
* [PATCH] tls: make tls_sw_free_resources static
From: Tobias Klauser @ 2017-09-14 11:22 UTC (permalink / raw)
To: Ilya Lesokhin, Aviad Yehezkel, Dave Watson; +Cc: David S. Miller, netdev
Make the needlessly global function tls_sw_free_resources static to fix
a gcc/sparse warning.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fa596fa71ba7..7d80040a37b6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -639,7 +639,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
return ret;
}
-void tls_sw_free_resources(struct sock *sk)
+static void tls_sw_free_resources(struct sock *sk)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
--
2.13.0
^ permalink raw reply related
* [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Valentin Longchamp @ 2017-09-14 12:05 UTC (permalink / raw)
To: leoli, netdev; +Cc: linuxppc-dev, Valentin Longchamp
Simple printk format warning for the the ucc registers address.
Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index f77ba9fa257b..56b8fdb35c3b 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
}
if (netif_msg_probe(&debug))
- pr_info("UCC%1d at 0x%8x (irq = %d)\n",
+ pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
ug_info->uf_info.ucc_num + 1, ug_info->uf_info.regs,
ug_info->uf_info.irq);
--
2.13.5
^ permalink raw reply related
* Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes
From: Leon Romanovsky @ 2017-09-14 12:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ilan Tayari, Saeed Mahameed, Matan Barak, Boris Pismenny,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170914110628.3590833-1-arnd-r2nGTMty4D4@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]
On Thu, Sep 14, 2017 at 01:06:18PM +0200, Arnd Bergmann wrote:
> calling mlx5_fpga_mem_{read,write}_i2c() with a zero length on
> older compiler version such as gcc-4.6 results in a warning that
> the return code is not initialized:
>
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:147:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:126:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
>
> On newer compilers, the 'err' variable is optimized away in this
> code path and assumed to be zero when the loop completes, so we
> don't get this warning.
>
> I'm changing the function here to instead return -EINVAL for the
> case, under the assumption that it was never meant to be called
> with a zero length argument.
I agree with you that size can't be zero and this patch will fix the
warning, but if it is possible, I will prefer to have this check is
written explicitly and not implicitly.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
index 3c11d6e2160a..0e85bebe1dad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
@@ -69,6 +69,9 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
if (!fdev->mdev)
return -ENOTCONN;
+ if (!size)
+ return -EINVAL;
+
while (bytes_done < size) {
actual_size = min(max_size, (size - bytes_done));
Thanks
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> index 3c11d6e2160a..914fb9d77a1a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> @@ -64,7 +64,7 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
> size_t bytes_done = 0;
> u8 actual_size;
> - int err;
> + int err = -EINVAL;
>
> if (!fdev->mdev)
> return -ENOTCONN;
> @@ -93,7 +93,7 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
> size_t bytes_done = 0;
> u8 actual_size;
> - int err;
> + int err = -EINVAL;
>
> if (!fdev->mdev)
> return -ENOTCONN;
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* Re: [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Christophe LEROY @ 2017-09-14 13:24 UTC (permalink / raw)
To: Valentin Longchamp, leoli, netdev; +Cc: linuxppc-dev
In-Reply-To: <20170914120514.31845-1-valentin.longchamp@keymile.com>
Hi,
Le 14/09/2017 à 14:05, Valentin Longchamp a écrit :
> Simple printk format warning for the the ucc registers address.
Did you test your patch with mpc83xx_defconfig ?
I get a new warning with your patch:
CC drivers/net/ethernet/freescale/ucc_geth.o
In file included from ./include/linux/printk.h:6:0,
from ./include/linux/kernel.h:13,
from drivers/net/ethernet/freescale/ucc_geth.c:18:
drivers/net/ethernet/freescale/ucc_geth.c: In function ‘ucc_geth_probe’:
./include/linux/kern_levels.h:4:18: warning: format ‘%llx’ expects
argument of type ‘long long unsigned int’, but argument 3 has type
‘resource_size_t {aka unsigned int}’ [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
./include/linux/kern_levels.h:13:19: note: in expansion of macro ‘KERN_SOH’
#define KERN_INFO KERN_SOH "6" /* informational */
^
./include/linux/printk.h:308:9: note: in expansion of macro ‘KERN_INFO’
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^
drivers/net/ethernet/freescale/ucc_geth.c:3860:3: note: in expansion of
macro ‘pr_info’
pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
^
Christophe
>
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index f77ba9fa257b..56b8fdb35c3b 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
> }
>
> if (netif_msg_probe(&debug))
> - pr_info("UCC%1d at 0x%8x (irq = %d)\n",
> + pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> ug_info->uf_info.ucc_num + 1, ug_info->uf_info.regs,
> ug_info->uf_info.irq);
>
>
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Neal Cardwell @ 2017-09-14 13:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: liujian, David Miller, Eric Dumazet, Yuchung Cheng, Jerry Chu,
Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <1505359839.15310.209.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> From: Eric Dumazet <edumazet@googl.com>
>
> liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
> in tcp_probe_timer() :
> https://www.spinics.net/lists/netdev/msg454496.html
>
> After investigations, the root cause of the problem is that we update
> skb->skb_mstamp of skbs in write queue, even if the attempt to send a
> clone or copy of it failed. One reason being a routing problem.
>
> This patch prevents this, solving liujian issue.
>
> It also removes a potential RTT miscalculation, since
> __tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
> TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
> been changed.
>
> A future ACK would then lead to a very small RTT sample and min_rtt
> would then be lowered to this too small value.
...
>
> Signed-off-by: Eric Dumazet <edumazet@googl.com>
> Reported-by: liujian <liujian56@huawei.com>
> ---
> net/ipv4/tcp_output.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: nixiaoming @ 2017-09-14 14:07 UTC (permalink / raw)
To: davem, edumazet, waltje, gw4pts, andreyknvl, tklauser,
philip.pettersson, glider, willemdebruijn.kernel
Cc: netdev, linux-kernel, nixiaoming, dede.wu
From: l00219569 <lisimin@huawei.com>
If fanout_add is preempted after running po-> fanout = match
and before running __fanout_link,
it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
or add spin_lock(&po->bind_lock) before po-> fanout = match
this is a patch for add po->bind_lock in fanout_add
test on linux 4.1.12:
./trinity -c setsockopt -C 2 -X &
BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
Kernel panic - not syncing: BUG!
CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
Call trace:
[<ffffffc000209414>] dump_backtrace+0x0/0xf8
[<ffffffc00020952c>] show_stack+0x20/0x28
[<ffffffc000635574>] dump_stack+0xac/0xe4
[<ffffffc000633fb8>] panic+0xf8/0x268
[<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
[<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
[<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
[<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
Signed-off-by: nixiaoming <nixiaoming@huawei.com>
Tested-by: wudesheng <dede.wu@huawei.com>
---
net/packet/af_packet.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 54a18a8..7a52a3b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1446,12 +1446,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
default:
return -EINVAL;
}
-
- if (!po->running)
+ spin_lock(&po->bind_lock);
+ if (!po->running) {
+ spin_unlock(&po->bind_lock);
return -EINVAL;
+ }
- if (po->fanout)
+ if (po->fanout) {
+ spin_unlock(&po->bind_lock);
return -EALREADY;
+ }
mutex_lock(&fanout_mutex);
match = NULL;
@@ -1501,6 +1505,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
}
out:
mutex_unlock(&fanout_mutex);
+ spin_unlock(&po->bind_lock);
return err;
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Longchamp, Valentin @ 2017-09-14 14:17 UTC (permalink / raw)
To: christophe.leroy@c-s.fr, netdev@vger.kernel.org,
leoli@freescale.com
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <f5c5c526-99d4-ff3d-1e2a-23d71bcafa4b@c-s.fr>
Hi Christophe,
On Thu, 2017-09-14 at 15:24 +0200, Christophe LEROY wrote:
> Hi,
>
> Le 14/09/2017 à 14:05, Valentin Longchamp a écrit :
> > Simple printk format warning for the the ucc registers address.
>
> Did you test your patch with mpc83xx_defconfig ?
No I only tested on a 85xx where I had another (similar, because the
physical addresses are u64 and not u32) warning.
My quick fix indeed did not take the different typedefs for
phys_addr_t.
I try to come with a v2 that covers this.
Thanks for the feedback.
Valentin
>
> I get a new warning with your patch:
>
> CC drivers/net/ethernet/freescale/ucc_geth.o
> In file included from ./include/linux/printk.h:6:0,
> from ./include/linux/kernel.h:13,
> from drivers/net/ethernet/freescale/ucc_geth.c:18:
> drivers/net/ethernet/freescale/ucc_geth.c: In function
> ‘ucc_geth_probe’:
> ./include/linux/kern_levels.h:4:18: warning: format ‘%llx’ expects
> argument of type ‘long long unsigned int’, but argument 3 has type
> ‘resource_size_t {aka unsigned int}’ [-Wformat=]
> #define KERN_SOH "\001" /* ASCII Start Of Header */
> ^
> ./include/linux/kern_levels.h:13:19: note: in expansion of macro
> ‘KERN_SOH’
> #define KERN_INFO KERN_SOH "6" /* informational */
> ^
> ./include/linux/printk.h:308:9: note: in expansion of macro
> ‘KERN_INFO’
> printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> ^
> drivers/net/ethernet/freescale/ucc_geth.c:3860:3: note: in expansion
> of
> macro ‘pr_info’
> pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> ^
>
> Christophe
>
> >
> > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> > ---
> > drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
> > b/drivers/net/ethernet/freescale/ucc_geth.c
> > index f77ba9fa257b..56b8fdb35c3b 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct
> > platform_device* ofdev)
> > }
> >
> > if (netif_msg_probe(&debug))
> > - pr_info("UCC%1d at 0x%8x (irq = %d)\n",
> > + pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> > ug_info->uf_info.ucc_num + 1, ug_info-
> > >uf_info.regs,
> > ug_info->uf_info.irq);
> >
> >
^ permalink raw reply
* Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
From: Andrew Lunn @ 2017-09-14 14:34 UTC (permalink / raw)
To: Fahad Kunnathadi
Cc: f.fainelli, netdev, michal.simek, linux-kernel, soren.brinkmann,
linux-arm-kernel
In-Reply-To: <1505373391-18697-1-git-send-email-fahad.kunnathadi@dexceldesigns.com>
On Thu, Sep 14, 2017 at 12:46:31PM +0530, Fahad Kunnathadi wrote:
> To clear Speed Selection in MDIO control register(0x10),
> ie, clear bits 6 and 13 to zero while keeping other bits same.
> Before AND operation,The Mask value has to be perform with bitwise NOT
> operation (ie, ~ operator)
>
> This patch clears current speed selection before writing the
> new speed settings to gmii2rgmii converter
Hi Fahad
I expect you will find other issues with this driver. I pointed some
out at the time it is submitted, but the developers went quiet as soon
as it was accepted.
Anyway, please ensure David Miller <davem@davemloft.net> gets a copy.
The subject line should be:
[PATCH net] net: phy: Fix mask value write on gmii2rgmii converter speed register.
and include a fixes tag:
Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-14 14:35 UTC (permalink / raw)
To: nixiaoming
Cc: David Miller, Eric Dumazet, waltje, gw4pts, Andrey Konovalov,
Tobias Klauser, Philip Pettersson, Alexander Potapenko,
Network Development, LKML, dede.wu
In-Reply-To: <20170914140722.44292-1-nixiaoming@huawei.com>
On Thu, Sep 14, 2017 at 10:07 AM, nixiaoming <nixiaoming@huawei.com> wrote:
> From: l00219569 <lisimin@huawei.com>
>
> If fanout_add is preempted after running po-> fanout = match
> and before running __fanout_link,
> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>
> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
The packet socket code has no shortage of locks, so there are many
ways to avoid the race condition between fanout_add and packet_set_ring.
Another option would be to lock the socket when calling fanout_add:
- return fanout_add(sk, val & 0xffff, val >> 16);
+ lock_sock(sk);
+ ret = fanout_add(sk, val & 0xffff, val >> 16);
+ release_sock(sk);
+ return ret;
But, for consistency, and to be able to continue to make sense of the
locking policy, we should use the most appropriate lock. This
is po->bind_lock, as it ensures atomicity between testing whether
a protocol hook is active through po->running and the actual existence
of that hook on the protocol hook list.
fanout_mutex protects the fanout object's list. Taking that on
__unregister_prot_hook even in the case where fanout is not
used (and __dev_remove_pack is called) complicates locking
in this already complicated code.
> or add spin_lock(&po->bind_lock) before po-> fanout = match
>
> this is a patch for add po->bind_lock in fanout_add
>
> test on linux 4.1.12:
> ./trinity -c setsockopt -C 2 -X &
Thanks for testing!
>
> BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
> Kernel panic - not syncing: BUG!
> CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
> Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
> Call trace:
> [<ffffffc000209414>] dump_backtrace+0x0/0xf8
> [<ffffffc00020952c>] show_stack+0x20/0x28
> [<ffffffc000635574>] dump_stack+0xac/0xe4
> [<ffffffc000633fb8>] panic+0xf8/0x268
> [<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
> [<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
> [<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
> [<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> Tested-by: wudesheng <dede.wu@huawei.com>
> ---
> net/packet/af_packet.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 54a18a8..7a52a3b 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1446,12 +1446,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> default:
> return -EINVAL;
> }
> -
> - if (!po->running)
> + spin_lock(&po->bind_lock);
> + if (!po->running) {
> + spin_unlock(&po->bind_lock);
> return -EINVAL;
> + }
>
> - if (po->fanout)
> + if (po->fanout) {
> + spin_unlock(&po->bind_lock);
> return -EALREADY;
> + }
>
> mutex_lock(&fanout_mutex);
> match = NULL;
> @@ -1501,6 +1505,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> }
> out:
> mutex_unlock(&fanout_mutex);
> + spin_unlock(&po->bind_lock);
This function can call kzalloc with GFP_KERNEL, which may sleep. It is
not correct to sleep while holding a spinlock. Which is why I take the lock
later and test po->running again.
I will clean up that patch and send it for review.
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
From: Waskiewicz Jr, Peter @ 2017-09-14 14:47 UTC (permalink / raw)
To: Brown, Aaron F, Christophe JAILLET, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C6A028B@ORSMSX101.amr.corp.intel.com>
On 9/13/17 7:24 PM, Brown, Aaron F wrote:
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of Christophe JAILLET
>> Sent: Monday, August 28, 2017 10:13 AM
>> To: Waskiewicz Jr, Peter <peter.waskiewicz.jr@intel.com>; Kirsher, Jeffrey T
>> <jeffrey.t.kirsher@intel.com>
>> Cc: netdev@vger.kernel.org; kernel-janitors@vger.kernel.org; intel-wired-
>> lan@lists.osuosl.org; linux-kernel@vger.kernel.org
>> Subject: Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
>>
>> Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :
>>> On 8/27/17 2:42 AM, Christophe JAILLET wrote:
>>>> Check memory allocation failures and return -ENOMEM in such cases, as
>>>> already done for other memory allocations in this function.
>>>>
>>>> This avoids NULL pointers dereference.
>>>>
>>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>>> ---
>>>> drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>
> This seems to be fine from a "it does not break in testing" perspective, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com
>
>>> -PJ
>>>
>> Hi,
>>
>> in fact, there is no leak because the only caller of 'igb_sw_init()'
>> (i.e. 'igb_probe()'), already frees these resources in case of error,
>> see [1]
>>
>> These resources are also freed in 'igb_remove()'.
>>
>> Best reagrds,
>> CJ
>>
>> [1]:
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
>> next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775
>
> But is PJ's comment saying that it is not really necessary? If so I tend to lean towards the don't touch it if it's not broken perspective.
I guess I didn't respond after Christophe replied, sorry about that.
The patch is good to me. It's definitely catching an issue where we're
not checking for a memory failure, then just follows the same
de-allocation path on unwind.
If you want it:
Acked-by: PJ Waskiewicz <peter.waskiewicz.jr@intel.com>
^ permalink raw reply
* [PATCH iproute2] tc: fix typo in tc-tcindex man page
From: Davide Caratti @ 2017-09-14 15:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
fix mis-typed 'pass_on' keyword.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
man/man8/tc-tcindex.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man/man8/tc-tcindex.8 b/man/man8/tc-tcindex.8
index 7fcf8254..9a4e5ffc 100644
--- a/man/man8/tc-tcindex.8
+++ b/man/man8/tc-tcindex.8
@@ -11,7 +11,7 @@ tcindex \- traffic control index filter
.IR MASK " ] [ "
.B shift
.IR SHIFT " ] [ "
-.BR pas_on " | " fall_through " ] [ " classid
+.BR pass_on " | " fall_through " ] [ " classid
.IR CLASSID " ] [ "
.B action
.BR ACTION_SPEC " ]"
--
2.13.5
^ permalink raw reply related
* [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Chen Haiquan @ 2017-09-14 15:14 UTC (permalink / raw)
To: jbenc; +Cc: davem, netdev, oc
The purpose of vxlan arp reduce feature is to reply the broadcast
arp request in vtep instead of sending it out to save traffic.
The current implementation drops arp packet, if the ip cannot be
found in neigh table. In the case of virtual IP address, user
defines IP address without management from SDN controller. The IP
address does not exist in neigh table, so the arp broadcast request
from a client can not be sent to the server who owns the virtual IP
address.
This patch allow the arp request to be sent out if:
1. not arp broadcast request
2. cannot be found in neigh table
3. arp record status is not NUD_CONNECTED
The user defined of virtual IP address works while arp reduce still
suppress the arp broadcast for IP address managed by SDN controller
with this patch.
Signed-off-by: Chen Haiquan <oc@yunify.com>
---
Changes in v2:
- Add config option, arp_reduce_ignore_unknown_ip, to enable
the behavior.
---
drivers/net/vxlan.c | 42 +++++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf1d5e9..cc9ee28f3481 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -53,6 +53,11 @@ static bool log_ecn_error = true;
module_param(log_ecn_error, bool, 0644);
MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+static bool arp_reduce_ignore_unknown_ip;
+module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
+MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
+ "Only reduce known arp broaddcast request to support virtual IP");
+
static unsigned int vxlan_net_id;
static struct rtnl_link_ops vxlan_link_ops;
@@ -1473,7 +1478,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
parp->ar_op != htons(ARPOP_REQUEST) ||
parp->ar_hln != dev->addr_len ||
parp->ar_pln != 4)
- goto out;
+ goto ignore;
arpptr = (u8 *)parp + sizeof(struct arphdr);
sha = arpptr;
arpptr += dev->addr_len; /* sha */
@@ -1494,7 +1499,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1526,10 +1531,20 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown arp */
+ goto ignore;
}
+
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -1642,7 +1657,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
msg = (struct nd_msg *)(iphdr + 1);
if (msg->icmph.icmp6_code != 0 ||
msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
- goto out;
+ goto ignore;
if (ipv6_addr_loopback(daddr) ||
ipv6_addr_is_multicast(&msg->target))
@@ -1656,7 +1671,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1684,11 +1699,20 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown neigh */
+ goto ignore;
}
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#endif
@@ -2266,8 +2290,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
- if (ntohs(eth->h_proto) == ETH_P_ARP)
- return arp_reduce(dev, skb, vni);
+ if (ntohs(eth->h_proto) == ETH_P_ARP) {
+ if (arp_reduce(dev, skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
+ }
#if IS_ENABLED(CONFIG_IPV6)
else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
struct ipv6hdr *hdr, _hdr;
@@ -2275,7 +2301,9 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
skb_network_offset(skb),
sizeof(_hdr), &_hdr)) &&
hdr->nexthdr == IPPROTO_ICMPV6)
- return neigh_reduce(dev, skb, vni);
+ if (neigh_reduce(dev,
+ skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
}
#endif
}
--
2.7.4
^ permalink raw reply related
* selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-14 15:33 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer
Cc: linux-kernel, linux-kselftest, Shuah Khan, Networking
In-Reply-To: <ed20875b-7a01-fbc8-e8d1-3929ba35c8ac@kernel.org>
Hi Alexei and Daniel,
bpf test depends on clang and fails to compile when
------------------------------------------------------
make -C tools/testing/selftests/bpf run_tests
make: clang: Command not found
Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
With "make TARGETS=bpf kselftest" it fails earlier:
make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
Makefile:40: tools/scripts/Makefile.arch: No such file or directory
Makefile:84: tools/build/Makefile.feature: No such file or directory
Makefile:143: tools/build/Makefile.include: No such file or directory
make[3]: *** No rule to make target 'tools/build/Makefile.include'. Stop.
make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
Makefile:69: recipe for target 'all' failed
make[1]: *** [all] Error 2
Makefile:1190: recipe for target 'kselftest' failed
make: *** [kselftest] Error 2
--------------------------------------------------------------
Is bpf test intended to be run in kselftest run? The clang dependency might
not be met on majority of the systems. Is this a hard dependency??
Would it make sense to create a special target for bpf test? We do have a few
tests that do that now.
TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug
I could add a special target for bpf TARGET_BPF perhaps and exclude it from
the run_tests.
Please let me know your thoughts on this.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH] tg3: clean up redundant initialization of tnapi
From: Colin King @ 2017-09-14 16:01 UTC (permalink / raw)
To: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
tnapi is being initialized and then immediately updated and
hence the initialiation is redundant. Clean up the warning
by moving the declaration and initialization to the inside
of the for-loop.
Cleans up clang scan-build warning:
warning: Value stored to 'tnapi' during its initialization is never read
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/broadcom/tg3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index af33dc15c55f..54588809b867 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -11536,11 +11536,11 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq,
tg3_napi_enable(tp);
for (i = 0; i < tp->irq_cnt; i++) {
- struct tg3_napi *tnapi = &tp->napi[i];
err = tg3_request_irq(tp, i);
if (err) {
for (i--; i >= 0; i--) {
- tnapi = &tp->napi[i];
+ struct tg3_napi *tnapi = &tp->napi[i];
+
free_irq(tnapi->irq_vec, tnapi);
}
goto out_napi_fini;
--
2.14.1
^ permalink raw reply related
* Re: [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Jiri Benc @ 2017-09-14 16:14 UTC (permalink / raw)
To: Chen Haiquan; +Cc: davem, netdev
In-Reply-To: <20170914151440.GA30675@samsung>
On Thu, 14 Sep 2017 23:14:40 +0800, Chen Haiquan wrote:
> +static bool arp_reduce_ignore_unknown_ip;
> +module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
> +MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
> + "Only reduce known arp broaddcast request to support virtual IP");
Oh, no. This is not the way. It needs to be a runtime configuration
option (a netlink attribute). Generally, every time you find yourself
adding a module parameter, you're doing something wrong.
Also, net-next is currently closed. You should not submit new features
during this period, it's bugfixes only now.
http://vger.kernel.org/~davem/net-next.html
Jiri
^ permalink raw reply
* [PATCH net] netvsc: increase default receive buffer size
From: Stephen Hemminger @ 2017-09-14 16:31 UTC (permalink / raw)
To: kys, haiyangz, davem; +Cc: netdev, Stephen Hemminger, devel
The default receive buffer size was reduced by recent change
to a value which was appropriate for 10G and Windows Server 2016.
But the value is too small for full performance with 40G on Azure.
Increase the default back to maximum supported by host.
Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c538a4f15f3b..d4902ee5f260 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -49,7 +49,7 @@
#define NETVSC_MIN_TX_SECTIONS 10
#define NETVSC_DEFAULT_TX 192 /* ~1M */
#define NETVSC_MIN_RX_SECTIONS 10 /* ~64K */
-#define NETVSC_DEFAULT_RX 2048 /* ~4M */
+#define NETVSC_DEFAULT_RX 10485 /* Max ~16M */
#define LINKCHANGE_INT (2 * HZ)
#define VF_TAKEOVER_INT (HZ / 10)
--
2.11.0
^ permalink raw reply related
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