* Re: [PATCH for-next 00/14][PULL request] Mellanox mlx5 core driver updates 2016-10-25
From: David Miller @ 2016-10-28 17:53 UTC (permalink / raw)
To: saeedm-VPRAkNaXOzVWk0Htik3J/w
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w, leonro-VPRAkNaXOzVWk0Htik3J/w,
talal-VPRAkNaXOzVWk0Htik3J/w, matanb-VPRAkNaXOzVWk0Htik3J/w
In-Reply-To: <1477407617-20745-1-git-send-email-saeedm-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
I really disalike pull requests of this form.
You add lots of datastructures and helper functions but no actual
users of these facilities to the driver.
Do this instead:
1) Add TSAR infrastructure
2) Add use of TSAR facilities to the driver
That's one pull request.
I don't care if this is hard, or if there are entanglements with
Infiniband or whatever, you must submit changes in this manner.
I will not accept additions to a driver that don't even get really
used.
--
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
* Re: [PATCH net-next] udp: do fwd memory scheduling on dequeue
From: Eric Dumazet @ 2016-10-28 17:50 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, James Morris, Trond Myklebust,
Alexander Duyck, Daniel Borkmann, Eric Dumazet, Tom Herbert,
Hannes Frederic Sowa, linux-nfs
In-Reply-To: <1477674975.7065.245.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, 2016-10-28 at 10:16 -0700, Eric Dumazet wrote:
> Nice !
>
> I was working on this as well and my implementation was somewhat
> different.
This is my WIP
Note this can be split in two parts.
1) One adding struct sock *sk param to ip_cmsg_recv_offset()
This was because I left skb->sk NULL for skbs stored in receive
queue.
You chose instead to set skb->sk, which is unusual (check
skb_orphan() BUG_ON())
2) Udp changes.
Tell me what you think, thanks again !
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 601258f6e621..52e70431abc9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3033,9 +3033,13 @@ int __skb_wait_for_more_packets(struct sock *sk, int *err, long *timeo_p,
const struct sk_buff *skb);
struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned flags,
int *peeked, int *off, int *err,
- struct sk_buff **last);
+ struct sk_buff **last,
+ void (*destructor)(struct sock *sk,
+ struct sk_buff *skb));
struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
- int *peeked, int *off, int *err);
+ int *peeked, int *off, int *err,
+ void (*destructor)(struct sock *sk,
+ struct sk_buff *skb));
struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags, int noblock,
int *err);
unsigned int datagram_poll(struct file *file, struct socket *sock,
diff --git a/include/net/ip.h b/include/net/ip.h
index 79b102ffbe16..f91fc376f3fb 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -572,7 +572,8 @@ int ip_options_rcv_srr(struct sk_buff *skb);
*/
void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb);
-void ip_cmsg_recv_offset(struct msghdr *msg, struct sk_buff *skb, int tlen, int offset);
+void ip_cmsg_recv_offset(struct msghdr *msg, struct sock *sk,
+ struct sk_buff *skb, int tlen, int offset);
int ip_cmsg_send(struct sock *sk, struct msghdr *msg,
struct ipcm_cookie *ipc, bool allow_ipv6);
int ip_setsockopt(struct sock *sk, int level, int optname, char __user *optval,
@@ -594,7 +595,7 @@ void ip_local_error(struct sock *sk, int err, __be32 daddr, __be16 dport,
static inline void ip_cmsg_recv(struct msghdr *msg, struct sk_buff *skb)
{
- ip_cmsg_recv_offset(msg, skb, 0, 0);
+ ip_cmsg_recv_offset(msg, skb->sk, skb, 0, 0);
}
bool icmp_global_allow(void);
diff --git a/include/net/udp.h b/include/net/udp.h
index 18f1e6b91927..0178f4552c4d 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -248,6 +248,7 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
/* net/ipv4/udp.c */
void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb);
+void udp_skb_destructor(struct sock *sk, struct sk_buff *skb);
void udp_v4_early_demux(struct sk_buff *skb);
int udp_get_port(struct sock *sk, unsigned short snum,
diff --git a/net/core/datagram.c b/net/core/datagram.c
index bfb973aebb5b..48228d15f33c 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -198,7 +198,8 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
*/
struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
int *peeked, int *off, int *err,
- struct sk_buff **last)
+ struct sk_buff **last,
+ void (*destructor)(struct sock *sk, struct sk_buff *skb))
{
struct sk_buff_head *queue = &sk->sk_receive_queue;
struct sk_buff *skb;
@@ -241,9 +242,11 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
}
atomic_inc(&skb->users);
- } else
+ } else {
__skb_unlink(skb, queue);
-
+ if (destructor)
+ destructor(sk, skb);
+ }
spin_unlock_irqrestore(&queue->lock, cpu_flags);
*off = _off;
return skb;
@@ -262,7 +265,9 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
EXPORT_SYMBOL(__skb_try_recv_datagram);
struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
- int *peeked, int *off, int *err)
+ int *peeked, int *off, int *err,
+ void (*destructor)(struct sock *sk,
+ struct sk_buff *skb))
{
struct sk_buff *skb, *last;
long timeo;
@@ -271,7 +276,7 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
do {
skb = __skb_try_recv_datagram(sk, flags, peeked, off, err,
- &last);
+ &last, destructor);
if (skb)
return skb;
@@ -290,7 +295,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned int flags,
int peeked, off = 0;
return __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, err);
+ &peeked, &off, err, NULL);
}
EXPORT_SYMBOL(skb_recv_datagram);
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index b8a2d63d1fb8..1de70870d0aa 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -153,11 +153,10 @@ static void ip_cmsg_recv_dstaddr(struct msghdr *msg, struct sk_buff *skb)
put_cmsg(msg, SOL_IP, IP_ORIGDSTADDR, sizeof(sin), &sin);
}
-void ip_cmsg_recv_offset(struct msghdr *msg, struct sk_buff *skb,
- int tlen, int offset)
+void ip_cmsg_recv_offset(struct msghdr *msg, struct sock *sk,
+ struct sk_buff *skb, int tlen, int offset)
{
- struct inet_sock *inet = inet_sk(skb->sk);
- unsigned int flags = inet->cmsg_flags;
+ unsigned int flags = inet_sk(sk)->cmsg_flags;
/* Ordered by supposed usage frequency */
if (flags & IP_CMSG_PKTINFO) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index bfa9609ba0f4..8ff7ee74a992 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1178,20 +1178,20 @@ static void udp_rmem_release(struct sock *sk, int size, int partial)
atomic_sub(size, &sk->sk_rmem_alloc);
- spin_lock_bh(&sk->sk_receive_queue.lock);
sk->sk_forward_alloc += size;
amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
sk->sk_forward_alloc -= amt;
- spin_unlock_bh(&sk->sk_receive_queue.lock);
if (amt)
__sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
}
-static void udp_rmem_free(struct sk_buff *skb)
+/* Note : called with sk_receive_queue.lock held */
+void udp_skb_destructor(struct sock *sk, struct sk_buff *skb)
{
- udp_rmem_release(skb->sk, skb->truesize, 1);
+ udp_rmem_release(sk, skb->truesize, 1);
}
+EXPORT_SYMBOL(udp_skb_destructor);
int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
{
@@ -1220,17 +1220,14 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
if (!__sk_mem_raise_allocated(sk, delta, amt, SK_MEM_RECV)) {
err = -ENOBUFS;
spin_unlock(&list->lock);
- goto uncharge_drop;
+ goto drop;
}
sk->sk_forward_alloc += delta;
}
-
+ atomic_add(size, &sk->sk_rmem_alloc);
sk->sk_forward_alloc -= size;
- /* the skb owner in now the udp socket */
- skb->sk = sk;
- skb->destructor = udp_rmem_free;
skb->dev = NULL;
sock_skb_set_dropcount(sk, skb);
@@ -1253,9 +1250,14 @@ EXPORT_SYMBOL_GPL(__udp_enqueue_schedule_skb);
static void udp_destruct_sock(struct sock *sk)
{
- /* reclaim completely the forward allocated memory */
- __skb_queue_purge(&sk->sk_receive_queue);
- udp_rmem_release(sk, 0, 0);
+ unsigned int total = 0;
+ struct sk_buff *skb;
+
+ while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+ total += skb->truesize;
+ kfree_skb(skb);
+ }
+ udp_rmem_release(sk, total, 0);
inet_sock_destruct(sk);
}
@@ -1287,12 +1289,11 @@ EXPORT_SYMBOL_GPL(skb_consume_udp);
*/
static int first_packet_length(struct sock *sk)
{
- struct sk_buff_head list_kill, *rcvq = &sk->sk_receive_queue;
+ struct sk_buff_head *rcvq = &sk->sk_receive_queue;
struct sk_buff *skb;
+ int total = 0;
int res;
- __skb_queue_head_init(&list_kill);
-
spin_lock_bh(&rcvq->lock);
while ((skb = skb_peek(rcvq)) != NULL &&
udp_lib_checksum_complete(skb)) {
@@ -1302,12 +1303,14 @@ static int first_packet_length(struct sock *sk)
IS_UDPLITE(sk));
atomic_inc(&sk->sk_drops);
__skb_unlink(skb, rcvq);
- __skb_queue_tail(&list_kill, skb);
+ total += skb->truesize;
+ kfree_skb(skb);
}
res = skb ? skb->len : -1;
+ if (total)
+ udp_rmem_release(sk, total, 1);
spin_unlock_bh(&rcvq->lock);
- __skb_queue_purge(&list_kill);
return res;
}
@@ -1363,7 +1366,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
try_again:
peeking = off = sk_peek_offset(sk, flags);
skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, &err);
+ &peeked, &off, &err, udp_skb_destructor);
if (!skb)
return err;
@@ -1420,7 +1423,7 @@ int udp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len, int noblock,
*addr_len = sizeof(*sin);
}
if (inet->cmsg_flags)
- ip_cmsg_recv_offset(msg, skb, sizeof(struct udphdr), off);
+ ip_cmsg_recv_offset(msg, sk, skb, sizeof(struct udphdr), off);
err = copied;
if (flags & MSG_TRUNC)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index a7700bbf6788..7e602b89c64d 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -344,7 +344,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
try_again:
peeking = off = sk_peek_offset(sk, flags);
skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
- &peeked, &off, &err);
+ &peeked, &off, &err, udp_skb_destructor);
if (!skb)
return err;
@@ -425,7 +425,7 @@ int udpv6_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
if (is_udp4) {
if (inet->cmsg_flags)
- ip_cmsg_recv_offset(msg, skb,
+ ip_cmsg_recv_offset(msg, sk, skb,
sizeof(struct udphdr), off);
} else {
if (np->rxopt.all)
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 145082e2ba36..8b995f88d000 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2114,7 +2114,7 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
skip = sk_peek_offset(sk, flags);
skb = __skb_try_recv_datagram(sk, flags, &peeked, &skip, &err,
- &last);
+ &last, NULL);
if (skb)
break;
^ permalink raw reply related
* Re: [PATCH v2] LSO feature added to Cadence GEM driver
From: David Miller @ 2016-10-28 17:49 UTC (permalink / raw)
To: rafalo; +Cc: nicolas.ferre, eric.dumazet, netdev, linux-kernel
In-Reply-To: <1477397130-24250-1-git-send-email-rafalo@cadence.com>
From: Rafal Ozieblo <rafalo@cadence.com>
Date: Tue, 25 Oct 2016 13:05:30 +0100
> New Cadence GEM hardware support Large Segment Offload (LSO):
> TCP segmentation offload (TSO) as well as UDP fragmentation
> offload (UFO). Support for those features was added to the driver.
>
> Signed-off-by: Rafal Ozieblo <rafalo@cadence.com>
Patch does not apply cleanly to net-next, please respin.
Also please fix your Subject line, it should be of the form:
[PATCH net-next v2] cadence: Add LSO support.
^ permalink raw reply
* Re: [PATCH] net caif: insert missing spaces in pr_* messages and unbreak multi-line strings
From: David Miller @ 2016-10-28 17:47 UTC (permalink / raw)
To: colin.king; +Cc: dmitry.tarnyagin, netdev, linux-kernel
In-Reply-To: <20161025111842.8110-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Tue, 25 Oct 2016 12:18:42 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> Some of the pr_* messages are missing spaces, so insert these and also
> unbreak multi-line literal strings in pr_* messages
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [patch net-next 0/5] mlxsw: small driver update
From: David Miller @ 2016-10-28 17:46 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1477573981-26546-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 27 Oct 2016 15:12:56 +0200
> For details, see individual patches.
Series applied, thanks.
^ permalink raw reply
* Re: [patch net 0/2] mlxsw: Couple of fixes
From: David Miller @ 2016-10-28 17:44 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, eladr, yotamg, nogahf, ogerlitz
In-Reply-To: <1477387557-5048-1-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 25 Oct 2016 11:25:55 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Couple of LPM tree management fixes.
Series applied, thanks Jiri.
^ permalink raw reply
* Re: [net-next PATCH 02/27] swiotlb-xen: Enforce return of DMA_ERROR_CODE in mapping function
From: Konrad Rzeszutek Wilk @ 2016-10-28 17:35 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, intel-wired-lan, linux-kernel, linux-mm, brouer, davem
In-Reply-To: <20161025153658.4815.84254.stgit@ahduyck-blue-test.jf.intel.com>
On Tue, Oct 25, 2016 at 11:36:58AM -0400, Alexander Duyck wrote:
> The mapping function should always return DMA_ERROR_CODE when a mapping has
> failed as this is what the DMA API expects when a DMA error has occurred.
> The current function for mapping a page in Xen was returning either
> DMA_ERROR_CODE or 0 depending on where it failed.
>
> On x86 DMA_ERROR_CODE is 0, but on other architectures such as ARM it is
> ~0. We need to make sure we return the same error value if either the
> mapping failed or the device is not capable of accessing the mapping.
>
> If we are returning DMA_ERROR_CODE as our error value we can drop the
> function for checking the error code as the default is to compare the
> return value against DMA_ERROR_CODE if no function is defined.
>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
I am pretty sure I gave an Ack. Any particular reason from dropping it
(if so, please add a comment under the --- of the reason).
Thanks.
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> arch/arm/xen/mm.c | 1 -
> arch/x86/xen/pci-swiotlb-xen.c | 1 -
> drivers/xen/swiotlb-xen.c | 18 ++++++------------
> include/xen/swiotlb-xen.h | 3 ---
> 4 files changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
> index d062f08..bd62d94 100644
> --- a/arch/arm/xen/mm.c
> +++ b/arch/arm/xen/mm.c
> @@ -186,7 +186,6 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
> EXPORT_SYMBOL(xen_dma_ops);
>
> static struct dma_map_ops xen_swiotlb_dma_ops = {
> - .mapping_error = xen_swiotlb_dma_mapping_error,
> .alloc = xen_swiotlb_alloc_coherent,
> .free = xen_swiotlb_free_coherent,
> .sync_single_for_cpu = xen_swiotlb_sync_single_for_cpu,
> diff --git a/arch/x86/xen/pci-swiotlb-xen.c b/arch/x86/xen/pci-swiotlb-xen.c
> index 0e98e5d..a9fafb5 100644
> --- a/arch/x86/xen/pci-swiotlb-xen.c
> +++ b/arch/x86/xen/pci-swiotlb-xen.c
> @@ -19,7 +19,6 @@
> int xen_swiotlb __read_mostly;
>
> static struct dma_map_ops xen_swiotlb_dma_ops = {
> - .mapping_error = xen_swiotlb_dma_mapping_error,
> .alloc = xen_swiotlb_alloc_coherent,
> .free = xen_swiotlb_free_coherent,
> .sync_single_for_cpu = xen_swiotlb_sync_single_for_cpu,
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index 87e6035..b8014bf 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -416,11 +416,12 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> /*
> * Ensure that the address returned is DMA'ble
> */
> - if (!dma_capable(dev, dev_addr, size)) {
> - swiotlb_tbl_unmap_single(dev, map, size, dir);
> - dev_addr = 0;
> - }
> - return dev_addr;
> + if (dma_capable(dev, dev_addr, size))
> + return dev_addr;
> +
> + swiotlb_tbl_unmap_single(dev, map, size, dir);
> +
> + return DMA_ERROR_CODE;
> }
> EXPORT_SYMBOL_GPL(xen_swiotlb_map_page);
>
> @@ -648,13 +649,6 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> }
> EXPORT_SYMBOL_GPL(xen_swiotlb_sync_sg_for_device);
>
> -int
> -xen_swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
> -{
> - return !dma_addr;
> -}
> -EXPORT_SYMBOL_GPL(xen_swiotlb_dma_mapping_error);
> -
> /*
> * Return whether the given device DMA address mask can be supported
> * properly. For example, if your device can only drive the low 24-bits
> diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
> index 7c35e27..a0083be 100644
> --- a/include/xen/swiotlb-xen.h
> +++ b/include/xen/swiotlb-xen.h
> @@ -51,9 +51,6 @@ extern void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> int nelems, enum dma_data_direction dir);
>
> extern int
> -xen_swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr);
> -
> -extern int
> xen_swiotlb_dma_supported(struct device *hwdev, u64 mask);
>
> extern int
>
^ permalink raw reply
* Re: [net-next PATCH 03/27] swiotlb: Add support for DMA_ATTR_SKIP_CPU_SYNC
From: Konrad Rzeszutek Wilk @ 2016-10-28 17:34 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, intel-wired-lan, linux-kernel, linux-mm, brouer, davem
In-Reply-To: <20161025153703.4815.13673.stgit@ahduyck-blue-test.jf.intel.com>
On Tue, Oct 25, 2016 at 11:37:03AM -0400, Alexander Duyck wrote:
> As a first step to making DMA_ATTR_SKIP_CPU_SYNC apply to architectures
> beyond just ARM I need to make it so that the swiotlb will respect the
> flag. In order to do that I also need to update the swiotlb-xen since it
> heavily makes use of the functionality.
>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
I am pretty sure I acked it the RFC. Was there a particular
reason (this is very different from the RFC?) you dropped my ACk?
Thanks.
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> drivers/xen/swiotlb-xen.c | 11 +++++++---
> include/linux/swiotlb.h | 6 ++++--
> lib/swiotlb.c | 48 +++++++++++++++++++++++++++------------------
> 3 files changed, 40 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index b8014bf..3d048af 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -405,7 +405,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> */
> trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
>
> - map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir);
> + map = swiotlb_tbl_map_single(dev, start_dma_addr, phys, size, dir,
> + attrs);
> if (map == SWIOTLB_MAP_ERROR)
> return DMA_ERROR_CODE;
>
> @@ -419,7 +420,8 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
> if (dma_capable(dev, dev_addr, size))
> return dev_addr;
>
> - swiotlb_tbl_unmap_single(dev, map, size, dir);
> + swiotlb_tbl_unmap_single(dev, map, size, dir,
> + attrs | DMA_ATTR_SKIP_CPU_SYNC);
>
> return DMA_ERROR_CODE;
> }
> @@ -445,7 +447,7 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
>
> /* NOTE: We use dev_addr here, not paddr! */
> if (is_xen_swiotlb_buffer(dev_addr)) {
> - swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> + swiotlb_tbl_unmap_single(hwdev, paddr, size, dir, attrs);
> return;
> }
>
> @@ -558,11 +560,12 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> start_dma_addr,
> sg_phys(sg),
> sg->length,
> - dir);
> + dir, attrs);
> if (map == SWIOTLB_MAP_ERROR) {
> dev_warn(hwdev, "swiotlb buffer is full\n");
> /* Don't panic here, we expect map_sg users
> to do proper error handling. */
> + attrs |= DMA_ATTR_SKIP_CPU_SYNC;
> xen_swiotlb_unmap_sg_attrs(hwdev, sgl, i, dir,
> attrs);
> sg_dma_len(sgl) = 0;
> diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
> index e237b6f..4517be9 100644
> --- a/include/linux/swiotlb.h
> +++ b/include/linux/swiotlb.h
> @@ -44,11 +44,13 @@ enum dma_sync_target {
> extern phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
> dma_addr_t tbl_dma_addr,
> phys_addr_t phys, size_t size,
> - enum dma_data_direction dir);
> + enum dma_data_direction dir,
> + unsigned long attrs);
>
> extern void swiotlb_tbl_unmap_single(struct device *hwdev,
> phys_addr_t tlb_addr,
> - size_t size, enum dma_data_direction dir);
> + size_t size, enum dma_data_direction dir,
> + unsigned long attrs);
>
> extern void swiotlb_tbl_sync_single(struct device *hwdev,
> phys_addr_t tlb_addr,
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 47aad37..b538d39 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -425,7 +425,8 @@ static void swiotlb_bounce(phys_addr_t orig_addr, phys_addr_t tlb_addr,
> phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
> dma_addr_t tbl_dma_addr,
> phys_addr_t orig_addr, size_t size,
> - enum dma_data_direction dir)
> + enum dma_data_direction dir,
> + unsigned long attrs)
> {
> unsigned long flags;
> phys_addr_t tlb_addr;
> @@ -526,7 +527,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
> */
> for (i = 0; i < nslots; i++)
> io_tlb_orig_addr[index+i] = orig_addr + (i << IO_TLB_SHIFT);
> - if (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)
> + if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> + (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
> swiotlb_bounce(orig_addr, tlb_addr, size, DMA_TO_DEVICE);
>
> return tlb_addr;
> @@ -539,18 +541,20 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
>
> static phys_addr_t
> map_single(struct device *hwdev, phys_addr_t phys, size_t size,
> - enum dma_data_direction dir)
> + enum dma_data_direction dir, unsigned long attrs)
> {
> dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start);
>
> - return swiotlb_tbl_map_single(hwdev, start_dma_addr, phys, size, dir);
> + return swiotlb_tbl_map_single(hwdev, start_dma_addr, phys, size,
> + dir, attrs);
> }
>
> /*
> * dma_addr is the kernel virtual address of the bounce buffer to unmap.
> */
> void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
> - size_t size, enum dma_data_direction dir)
> + size_t size, enum dma_data_direction dir,
> + unsigned long attrs)
> {
> unsigned long flags;
> int i, count, nslots = ALIGN(size, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT;
> @@ -561,6 +565,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
> * First, sync the memory before unmapping the entry
> */
> if (orig_addr != INVALID_PHYS_ADDR &&
> + !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> ((dir == DMA_FROM_DEVICE) || (dir == DMA_BIDIRECTIONAL)))
> swiotlb_bounce(orig_addr, tlb_addr, size, DMA_FROM_DEVICE);
>
> @@ -654,7 +659,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
> * GFP_DMA memory; fall back on map_single(), which
> * will grab memory from the lowest available address range.
> */
> - phys_addr_t paddr = map_single(hwdev, 0, size, DMA_FROM_DEVICE);
> + phys_addr_t paddr = map_single(hwdev, 0, size,
> + DMA_FROM_DEVICE, 0);
> if (paddr == SWIOTLB_MAP_ERROR)
> goto err_warn;
>
> @@ -669,7 +675,8 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
>
> /* DMA_TO_DEVICE to avoid memcpy in unmap_single */
> swiotlb_tbl_unmap_single(hwdev, paddr,
> - size, DMA_TO_DEVICE);
> + size, DMA_TO_DEVICE,
> + DMA_ATTR_SKIP_CPU_SYNC);
> goto err_warn;
> }
> }
> @@ -699,7 +706,7 @@ void swiotlb_tbl_sync_single(struct device *hwdev, phys_addr_t tlb_addr,
> free_pages((unsigned long)vaddr, get_order(size));
> else
> /* DMA_TO_DEVICE to avoid memcpy in swiotlb_tbl_unmap_single */
> - swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE);
> + swiotlb_tbl_unmap_single(hwdev, paddr, size, DMA_TO_DEVICE, 0);
> }
> EXPORT_SYMBOL(swiotlb_free_coherent);
>
> @@ -755,7 +762,7 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
> trace_swiotlb_bounced(dev, dev_addr, size, swiotlb_force);
>
> /* Oh well, have to allocate and map a bounce buffer. */
> - map = map_single(dev, phys, size, dir);
> + map = map_single(dev, phys, size, dir, attrs);
> if (map == SWIOTLB_MAP_ERROR) {
> swiotlb_full(dev, size, dir, 1);
> return phys_to_dma(dev, io_tlb_overflow_buffer);
> @@ -764,12 +771,13 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
> dev_addr = phys_to_dma(dev, map);
>
> /* Ensure that the address returned is DMA'ble */
> - if (!dma_capable(dev, dev_addr, size)) {
> - swiotlb_tbl_unmap_single(dev, map, size, dir);
> - return phys_to_dma(dev, io_tlb_overflow_buffer);
> - }
> + if (dma_capable(dev, dev_addr, size))
> + return dev_addr;
> +
> + swiotlb_tbl_unmap_single(dev, map, size, dir,
> + attrs | DMA_ATTR_SKIP_CPU_SYNC);
>
> - return dev_addr;
> + return phys_to_dma(dev, io_tlb_overflow_buffer);
> }
> EXPORT_SYMBOL_GPL(swiotlb_map_page);
>
> @@ -782,14 +790,15 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
> * whatever the device wrote there.
> */
> static void unmap_single(struct device *hwdev, dma_addr_t dev_addr,
> - size_t size, enum dma_data_direction dir)
> + size_t size, enum dma_data_direction dir,
> + unsigned long attrs)
> {
> phys_addr_t paddr = dma_to_phys(hwdev, dev_addr);
>
> BUG_ON(dir == DMA_NONE);
>
> if (is_swiotlb_buffer(paddr)) {
> - swiotlb_tbl_unmap_single(hwdev, paddr, size, dir);
> + swiotlb_tbl_unmap_single(hwdev, paddr, size, dir, attrs);
> return;
> }
>
> @@ -809,7 +818,7 @@ void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> size_t size, enum dma_data_direction dir,
> unsigned long attrs)
> {
> - unmap_single(hwdev, dev_addr, size, dir);
> + unmap_single(hwdev, dev_addr, size, dir, attrs);
> }
> EXPORT_SYMBOL_GPL(swiotlb_unmap_page);
>
> @@ -891,7 +900,7 @@ void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> if (swiotlb_force ||
> !dma_capable(hwdev, dev_addr, sg->length)) {
> phys_addr_t map = map_single(hwdev, sg_phys(sg),
> - sg->length, dir);
> + sg->length, dir, attrs);
> if (map == SWIOTLB_MAP_ERROR) {
> /* Don't panic here, we expect map_sg users
> to do proper error handling. */
> @@ -925,7 +934,8 @@ void swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
> BUG_ON(dir == DMA_NONE);
>
> for_each_sg(sgl, sg, nelems, i)
> - unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir);
> + unmap_single(hwdev, sg->dma_address, sg_dma_len(sg), dir,
> + attrs);
>
> }
> EXPORT_SYMBOL(swiotlb_unmap_sg_attrs);
>
^ permalink raw reply
* Re: pull request (net-next): ipsec-next 2016-10-25
From: David Miller @ 2016-10-28 17:27 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <1477371941-13336-1-git-send-email-steffen.klassert@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 25 Oct 2016 07:05:40 +0200
> Just a leftover from the last development cycle.
>
> 1) Remove some unused code, from Florian Westphal.
>
> Please pull or let me know if there are problems.
Pulled, thanks!
^ permalink raw reply
* Re: [PATCH net-next] udp: do fwd memory scheduling on dequeue
From: Eric Dumazet @ 2016-10-28 17:16 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, David S. Miller, James Morris, Trond Myklebust,
Alexander Duyck, Daniel Borkmann, Eric Dumazet, Tom Herbert,
Hannes Frederic Sowa, linux-nfs
In-Reply-To: <95bb1b780be2e35ff04fb9e1e2c41470a0a15582.1477660091.git.pabeni@redhat.com>
Nice !
I was working on this as well and my implementation was somewhat
different.
I then stopped when I found the UDP checksum bug and was waiting for net
to be merged into net-next
( https://git.kernel.org/cgit/linux/kernel/git/davem/net.git/commit/?id=10df8e6152c6c400a563a673e9956320bfce1871 )
On Fri, 2016-10-28 at 15:20 +0200, Paolo Abeni wrote:
> A new argument is added to __skb_recv_datagram, it allows
> the caller to perform protocol specific action on dequeue
> under the receive queue spinlock.
> The UDP protocol uses such argument to perform fwd memory
> reclaiming on dequeue, while protocol memory and rmem updatating
> is delayed after the lock release, to keep the time spent
> under the lock as low as possible.
> The UDP specific skb desctructor is not used anymore, instead
> explicit memory reclaiming is performed at close() time and
> when skbs are removed from the receive queue.
> The in kernel UDP procotol users now need to use an
> skb_recv_udp() variant instead of skb_recv_datagram() to
> properly perform memory accounting on dequeue.
>
> Overall, this allows acquiring only once the receive queue
> lock on dequeue.
>
> Tested using pktgen with random src port, 64 bytes packet,
> wire-speed on a 10G link as sender and udp_sink as the receiver,
> using an l4 tuple rxhash to stress the contention, and one or more
> udp_sink instances with reuseport.
>
> nr sinks vanilla patched
> 1 440 560
> 3 2150 2300
> 6 3650 3800
> 9 4450 4600
> 12 6250 6450
>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> include/linux/skbuff.h | 4 ++++
> include/net/udp.h | 10 ++++++++
> net/core/datagram.c | 11 ++++++---
> net/ipv4/udp.c | 65 ++++++++++++++++++++++++++++++++++++++++----------
> net/ipv6/udp.c | 3 +--
> net/rxrpc/input.c | 7 +++---
> net/sunrpc/svcsock.c | 2 +-
> net/sunrpc/xprtsock.c | 2 +-
> net/unix/af_unix.c | 4 ++--
> 9 files changed, 83 insertions(+), 25 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 601258f..dd171a9 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3028,13 +3028,17 @@ static inline void skb_frag_list_init(struct sk_buff *skb)
> #define skb_walk_frags(skb, iter) \
> for (iter = skb_shinfo(skb)->frag_list; iter; iter = iter->next)
>
> +typedef void (*skb_dequeue_cb_t)(struct sock *sk, struct sk_buff *skb,
> + int flags);
>
> int __skb_wait_for_more_packets(struct sock *sk, int *err, long *timeo_p,
> const struct sk_buff *skb);
> struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned flags,
> + skb_dequeue_cb_t dequeue_cb,
> int *peeked, int *off, int *err,
> struct sk_buff **last);
> struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
> + skb_dequeue_cb_t dequeue_cb,
> int *peeked, int *off, int *err);
> struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned flags, int noblock,
> int *err);
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 18f1e6b..983c861 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -48,6 +48,7 @@ struct udp_skb_cb {
> } header;
> __u16 cscov;
> __u8 partial_cov;
> + int fwd_memory_released;
I was not adding this field.
> };
> #define UDP_SKB_CB(__skb) ((struct udp_skb_cb *)((__skb)->cb))
>
> @@ -248,6 +249,15 @@ static inline __be16 udp_flow_src_port(struct net *net, struct sk_buff *skb,
> /* net/ipv4/udp.c */
> void skb_consume_udp(struct sock *sk, struct sk_buff *skb, int len);
> int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb);
> +struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags, int noblock,
> + int *peeked, int *off, int *err);
> +static inline struct sk_buff *skb_recv_udp(struct sock *sk, unsigned int flags,
> + int noblock, int *err)
> +{
> + int peeked, off = 0;
> +
> + return __skb_recv_udp(sk, flags, noblock, &peeked, &off, err);
> +}
>
> void udp_v4_early_demux(struct sk_buff *skb);
> int udp_get_port(struct sock *sk, unsigned short snum,
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index bfb973a..226548b 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -165,6 +165,7 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
> * __skb_try_recv_datagram - Receive a datagram skbuff
> * @sk: socket
> * @flags: MSG_ flags
> + * @dequeue_cb: invoked under the receive lock on successful dequeue
> * @peeked: returns non-zero if this packet has been seen before
> * @off: an offset in bytes to peek skb from. Returns an offset
> * within an skb where data actually starts
> @@ -197,6 +198,7 @@ static struct sk_buff *skb_set_peeked(struct sk_buff *skb)
> * the standard around please.
> */
> struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
> + skb_dequeue_cb_t dequeue_cb,
> int *peeked, int *off, int *err,
> struct sk_buff **last)
> {
> @@ -244,6 +246,8 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
> } else
> __skb_unlink(skb, queue);
>
> + if (dequeue_cb)
> + dequeue_cb(sk, skb, flags);
I was doing this only if the packet was unlinked few lines above.
> spin_unlock_irqrestore(&queue->lock, cpu_flags);
> *off = _off;
> return skb;
> @@ -262,6 +266,7 @@ struct sk_buff *__skb_try_recv_datagram(struct sock *sk, unsigned int flags,
> EXPORT_SYMBOL(__skb_try_recv_datagram);
>
> struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
> + skb_dequeue_cb_t dequeue_cb,
> int *peeked, int *off, int *err)
> {
> struct sk_buff *skb, *last;
> @@ -270,8 +275,8 @@ struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned int flags,
> timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
>
> do {
> - skb = __skb_try_recv_datagram(sk, flags, peeked, off, err,
> - &last);
> + skb = __skb_try_recv_datagram(sk, flags, dequeue_cb, peeked,
> + off, err, &last);
> if (skb)
> return skb;
>
> @@ -290,7 +295,7 @@ struct sk_buff *skb_recv_datagram(struct sock *sk, unsigned int flags,
> int peeked, off = 0;
>
> return __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
> - &peeked, &off, err);
> + NULL, &peeked, &off, err);
> }
> EXPORT_SYMBOL(skb_recv_datagram);
>
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index c833271..2f1a727 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1172,26 +1172,61 @@ int udp_sendpage(struct sock *sk, struct page *page, int offset,
> return ret;
> }
>
> +/* fully reclaim rmem/fwd memory allocated for skb */
> static void udp_rmem_release(struct sock *sk, int size, int partial)
> {
> int amt;
>
> atomic_sub(size, &sk->sk_rmem_alloc);
> -
> - spin_lock_bh(&sk->sk_receive_queue.lock);
> sk->sk_forward_alloc += size;
> amt = (sk->sk_forward_alloc - partial) & ~(SK_MEM_QUANTUM - 1);
> sk->sk_forward_alloc -= amt;
> - spin_unlock_bh(&sk->sk_receive_queue.lock);
>
> if (amt)
> __sk_mem_reduce_allocated(sk, amt >> SK_MEM_QUANTUM_SHIFT);
> }
>
> -static void udp_rmem_free(struct sk_buff *skb)
> +/* if we are not peeking the skb, reclaim fwd allocated memory;
> + * rmem and protocol memory updating is delayed outside the lock
> + */
> +static void udp_dequeue(struct sock *sk, struct sk_buff *skb, int flags)
> +{
> + int amt;
> +
> + if (flags & MSG_PEEK)
> + return;
> +
> + sk->sk_forward_alloc += skb->truesize;
> + amt = (sk->sk_forward_alloc - 1) & ~(SK_MEM_QUANTUM - 1);
> + sk->sk_forward_alloc -= amt;
> + UDP_SKB_CB(skb)->fwd_memory_released = amt >> SK_MEM_QUANTUM_SHIFT;
> +}
> +
> +/* complete the memory reclaiming started with udp_dequeue */
> +static void __udp_rmem_release(struct sock *sk, struct sk_buff *skb, int flags)
> +{
> + int amt = UDP_SKB_CB(skb)->fwd_memory_released;
> +
> + if (flags & MSG_PEEK)
> + return;
> +
> + atomic_sub(skb->truesize, &sk->sk_rmem_alloc);
> + if (amt)
> + __sk_mem_reduce_allocated(sk, amt);
> +}
> +
> +struct sk_buff *__skb_recv_udp(struct sock *sk, unsigned int flags,
> + int noblock, int *peeked, int *off, int *err)
> {
> - udp_rmem_release(skb->sk, skb->truesize, 1);
> + struct sk_buff *skb;
> +
> + skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
> + udp_dequeue, peeked, off, err);
> + if (skb)
> + __udp_rmem_release(sk, skb, flags);
> + return skb;
> }
> +EXPORT_SYMBOL(__skb_recv_udp);
>
> int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> {
> @@ -1230,7 +1265,6 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
>
> /* the skb owner in now the udp socket */
> skb->sk = sk;
> - skb->destructor = udp_rmem_free;
> skb->dev = NULL;
> sock_skb_set_dropcount(sk, skb);
>
> @@ -1254,8 +1288,13 @@ int __udp_enqueue_schedule_skb(struct sock *sk, struct sk_buff *skb)
> static void udp_destruct_sock(struct sock *sk)
> {
> /* reclaim completely the forward allocated memory */
> - __skb_queue_purge(&sk->sk_receive_queue);
> - udp_rmem_release(sk, 0, 0);
> + struct sk_buff *skb;
> +
> + while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
> + udp_rmem_release(sk, skb->truesize, 0);
> + kfree_skb(skb);
> + }
> +
I was using instead :
+ unsigned int total = 0;
+ struct sk_buff *skb;
+
+ while ((skb = __skb_dequeue(&sk->sk_receive_queue)) != NULL) {
+ total += skb->truesize;
+ kfree_skb(skb);
+ }
+ udp_rmem_release(sk, total, 0);
> inet_sock_destruct(sk);
> }
>
> @@ -1303,11 +1342,11 @@ static int first_packet_length(struct sock *sk)
> atomic_inc(&sk->sk_drops);
> __skb_unlink(skb, rcvq);
> __skb_queue_tail(&list_kill, skb);
> + udp_rmem_release(sk, skb->truesize, 1);
> + kfree_skb(skb);
> }
> res = skb ? skb->len : -1;
> spin_unlock_bh(&rcvq->lock);
> -
^ permalink raw reply
* Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: David Miller @ 2016-10-28 17:11 UTC (permalink / raw)
To: john.fastabend
Cc: alexander.duyck, mst, brouer, shrijeet, tom, netdev, shm, roopa,
nikolay
In-Reply-To: <58137533.4030105@gmail.com>
From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 28 Oct 2016 08:56:35 -0700
> On 16-10-27 07:10 PM, David Miller wrote:
>> From: Alexander Duyck <alexander.duyck@gmail.com>
>> Date: Thu, 27 Oct 2016 18:43:59 -0700
>>
>>> On Thu, Oct 27, 2016 at 6:35 PM, David Miller <davem@davemloft.net> wrote:
>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>>> Date: Fri, 28 Oct 2016 01:25:48 +0300
>>>>
>>>>> On Thu, Oct 27, 2016 at 05:42:18PM -0400, David Miller wrote:
>>>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>>>>> Date: Fri, 28 Oct 2016 00:30:35 +0300
>>>>>>
>>>>>>> Something I'd like to understand is how does XDP address the
>>>>>>> problem that 100Byte packets are consuming 4K of memory now.
>>>>>>
>>>>>> Via page pools. We're going to make a generic one, but right now
>>>>>> each and every driver implements a quick list of pages to allocate
>>>>>> from (and thus avoid the DMA man/unmap overhead, etc.)
>>>>>
>>>>> So to clarify, ATM virtio doesn't attempt to avoid dma map/unmap
>>>>> so there should be no issue with that even when using sub/page
>>>>> regions, assuming DMA APIs support sub-page map/unmap correctly.
>>>>
>>>> That's not what I said.
>>>>
>>>> The page pools are meant to address the performance degradation from
>>>> going to having one packet per page for the sake of XDP's
>>>> requirements.
>>>>
>>>> You still need to have one packet per page for correct XDP operation
>>>> whether you do page pools or not, and whether you have DMA mapping
>>>> (or it's equivalent virutalization operation) or not.
>>>
>>> Maybe I am missing something here, but why do you need to limit things
>>> to one packet per page for correct XDP operation? Most of the drivers
>>> out there now are usually storing something closer to at least 2
>>> packets per page, and with the DMA API fixes I am working on there
>>> should be no issue with changing the contents inside those pages since
>>> we won't invalidate or overwrite the data after the DMA buffer has
>>> been synchronized for use by the CPU.
>>
>> Because with SKB's you can share the page with other packets.
>>
>> With XDP you simply cannot.
>>
>> It's software semantics that are the issue. SKB frag list pages
>> are read only, XDP packets are writable.
>>
>> This has nothing to do with "writability" of the pages wrt. DMA
>> mapping or cpu mappings.
>>
>
> Sorry I'm not seeing it either. The current xdp_buff is defined
> by,
>
> struct xdp_buff {
> void *data;
> void *data_end;
> };
>
> The verifier has an xdp_is_valid_access() check to ensure we don't go
> past data_end. The page for now at least never leaves the driver. For
> the work to get xmit to other devices working I'm still not sure I see
> any issue.
I guess I can say that the packets must be "writable" until I'm blue
in the face but I'll say it again, semantically writable pages are a
requirement. And if multiple packets share a page this requirement
is not satisfied.
Also, we want to do several things in the future:
1) Allow push/pop of headers via eBPF code, which needs we need
headroom.
2) Transparently zero-copy pass packets into userspace, basically
the user will have a semi-permanently mapped ring of all the
packet pages sitting in the RX queue of the device and the
page pool associated with it. This way we avoid all of the
TLB flush/map overhead for the user's mapping of the packets
just as we avoid the DMA map/unmap overhead.
And that's just the beginninng.
I'm sure others can come up with more reasons why we have this
requirement.
^ permalink raw reply
* Re: [net-next PATCH 00/27] Add support for DMA writable pages being writable by the network stack
From: David Miller @ 2016-10-28 17:06 UTC (permalink / raw)
To: alexander.duyck
Cc: netdev, intel-wired-lan, linux-kernel, linux-mm, brouer,
alexander.h.duyck, konrad.wilk, jeffrey.t.kirsher
In-Reply-To: <CAKgT0UfOZuRnon84_8Bdn5muoi7=Xrwd7Kbxi4C8jiXpyX7-gg@mail.gmail.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Fri, 28 Oct 2016 08:48:01 -0700
> So the feedback for this set has been mostly just a few "Acked-by"s,
> and it looks like the series was marked as "Not Applicable" in
> patchwork. I was wondering what the correct merge strategy for this
> patch set should be going forward?
I marked it as not applicable because it's definitely not a networking
change, and merging it via my tree would be really inappropriate, even
though we need it for some infrastructure we want to build for
networking.
So you have to merge this upstream via a more appropriate path.
> I was wondering if I should be looking at breaking up the set and
> splitting it over a few different trees, or if I should just hold onto
> it and resubmit it when the merge window opens? My preference would
> be to submit it as a single set so I can know all the patches are
> present to avoid any possible regressions due to only part of the set
> being present.
I don't think you need to split it up.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net 2/2] geneve: avoid using stale geneve socket.
From: Pravin Shelar @ 2016-10-28 17:01 UTC (permalink / raw)
To: John W. Linville; +Cc: Linux Kernel Network Developers
In-Reply-To: <20161028142743.GB31148@tuxdriver.com>
On Fri, Oct 28, 2016 at 7:27 AM, John W. Linville
<linville@tuxdriver.com> wrote:
> On Thu, Oct 27, 2016 at 11:51:56AM -0700, Pravin B Shelar wrote:
>> This patch is similar to earlier vxlan patch.
>> Geneve device close operation frees geneve socket. This
>> operation can race with geneve-xmit function which
>> dereferences geneve socket. Following patch uses RCU
>> mechanism to avoid this situation.
>>
>> Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
>
> LGTM, although I reckon that Stephen's comment about RCU_INIT_POINTER
> applies to this patch as well.
>
Geneve code is bit different and it is using rcu_assign_pointer()
while destroying the object. so we need the barrier.
> Either way...
>
> Acked-by: John W. Linville <linville@tuxdriver.com>
>
^ permalink raw reply
* Re: [PATCH net 1/2] vxlan: avoid using stale vxlan socket.
From: Pravin Shelar @ 2016-10-28 16:59 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Linux Kernel Network Developers
In-Reply-To: <20161027160219.06a946f4@xeon-e3>
On Thu, Oct 27, 2016 at 4:02 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 27 Oct 2016 11:51:55 -0700
> Pravin B Shelar <pshelar@ovn.org> wrote:
>
>> - vxlan->vn4_sock = NULL;
>> + rcu_assign_pointer(vxlan->vn4_sock, NULL);
>> #if IS_ENABLED(CONFIG_IPV6)
>> - vxlan->vn6_sock = NULL;
>> + rcu_assign_pointer(vxlan->vn6_sock, NULL);
>> if (ipv6 || metadata)
>
> User RCU_INIT_POINTER when setting pointer to NULL.
> It avoids unnecessary barrier.
OK. I have posted v2 patch to use RCU_INIT_POINTER().
^ permalink raw reply
* [PATCH v2 net 2/2] geneve: avoid using stale geneve socket.
From: Pravin B Shelar @ 2016-10-28 16:59 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
In-Reply-To: <1477673956-127319-1-git-send-email-pshelar@ovn.org>
This patch is similar to earlier vxlan patch.
Geneve device close operation frees geneve socket. This
operation can race with geneve-xmit function which
dereferences geneve socket. Following patch uses RCU
mechanism to avoid this situation.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
Acked-by: John W. Linville <linville@tuxdriver.com>
---
drivers/net/geneve.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
index 16af1ce..42edd7b 100644
--- a/drivers/net/geneve.c
+++ b/drivers/net/geneve.c
@@ -58,9 +58,9 @@ struct geneve_dev {
struct hlist_node hlist; /* vni hash table */
struct net *net; /* netns for packet i/o */
struct net_device *dev; /* netdev for geneve tunnel */
- struct geneve_sock *sock4; /* IPv4 socket used for geneve tunnel */
+ struct geneve_sock __rcu *sock4; /* IPv4 socket used for geneve tunnel */
#if IS_ENABLED(CONFIG_IPV6)
- struct geneve_sock *sock6; /* IPv6 socket used for geneve tunnel */
+ struct geneve_sock __rcu *sock6; /* IPv6 socket used for geneve tunnel */
#endif
u8 vni[3]; /* virtual network ID for tunnel */
u8 ttl; /* TTL override */
@@ -543,9 +543,19 @@ static void __geneve_sock_release(struct geneve_sock *gs)
static void geneve_sock_release(struct geneve_dev *geneve)
{
- __geneve_sock_release(geneve->sock4);
+ struct geneve_sock *gs4 = rtnl_dereference(geneve->sock4);
#if IS_ENABLED(CONFIG_IPV6)
- __geneve_sock_release(geneve->sock6);
+ struct geneve_sock *gs6 = rtnl_dereference(geneve->sock6);
+
+ rcu_assign_pointer(geneve->sock6, NULL);
+#endif
+
+ rcu_assign_pointer(geneve->sock4, NULL);
+ synchronize_net();
+
+ __geneve_sock_release(gs4);
+#if IS_ENABLED(CONFIG_IPV6)
+ __geneve_sock_release(gs6);
#endif
}
@@ -586,10 +596,10 @@ static int geneve_sock_add(struct geneve_dev *geneve, bool ipv6)
gs->flags = geneve->flags;
#if IS_ENABLED(CONFIG_IPV6)
if (ipv6)
- geneve->sock6 = gs;
+ rcu_assign_pointer(geneve->sock6, gs);
else
#endif
- geneve->sock4 = gs;
+ rcu_assign_pointer(geneve->sock4, gs);
hash = geneve_net_vni_hash(geneve->vni);
hlist_add_head_rcu(&geneve->hlist, &gs->vni_list[hash]);
@@ -603,9 +613,7 @@ static int geneve_open(struct net_device *dev)
bool metadata = geneve->collect_md;
int ret = 0;
- geneve->sock4 = NULL;
#if IS_ENABLED(CONFIG_IPV6)
- geneve->sock6 = NULL;
if (ipv6 || metadata)
ret = geneve_sock_add(geneve, true);
#endif
@@ -720,6 +728,9 @@ static struct rtable *geneve_get_v4_rt(struct sk_buff *skb,
struct rtable *rt = NULL;
__u8 tos;
+ if (!rcu_dereference(geneve->sock4))
+ return ERR_PTR(-EIO);
+
memset(fl4, 0, sizeof(*fl4));
fl4->flowi4_mark = skb->mark;
fl4->flowi4_proto = IPPROTO_UDP;
@@ -772,11 +783,15 @@ static struct dst_entry *geneve_get_v6_dst(struct sk_buff *skb,
{
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs6 = geneve->sock6;
struct dst_entry *dst = NULL;
struct dst_cache *dst_cache;
+ struct geneve_sock *gs6;
__u8 prio;
+ gs6 = rcu_dereference(geneve->sock6);
+ if (!gs6)
+ return ERR_PTR(-EIO);
+
memset(fl6, 0, sizeof(*fl6));
fl6->flowi6_mark = skb->mark;
fl6->flowi6_proto = IPPROTO_UDP;
@@ -842,7 +857,7 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
struct ip_tunnel_info *info)
{
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs4 = geneve->sock4;
+ struct geneve_sock *gs4;
struct rtable *rt = NULL;
const struct iphdr *iip; /* interior IP header */
int err = -EINVAL;
@@ -853,6 +868,10 @@ static netdev_tx_t geneve_xmit_skb(struct sk_buff *skb, struct net_device *dev,
bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
u32 flags = geneve->flags;
+ gs4 = rcu_dereference(geneve->sock4);
+ if (!gs4)
+ goto tx_error;
+
if (geneve->collect_md) {
if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
netdev_dbg(dev, "no tunnel metadata\n");
@@ -932,9 +951,9 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
struct ip_tunnel_info *info)
{
struct geneve_dev *geneve = netdev_priv(dev);
- struct geneve_sock *gs6 = geneve->sock6;
struct dst_entry *dst = NULL;
const struct iphdr *iip; /* interior IP header */
+ struct geneve_sock *gs6;
int err = -EINVAL;
struct flowi6 fl6;
__u8 prio, ttl;
@@ -943,6 +962,10 @@ static netdev_tx_t geneve6_xmit_skb(struct sk_buff *skb, struct net_device *dev,
bool xnet = !net_eq(geneve->net, dev_net(geneve->dev));
u32 flags = geneve->flags;
+ gs6 = rcu_dereference(geneve->sock6);
+ if (!gs6)
+ goto tx_error;
+
if (geneve->collect_md) {
if (unlikely(!info || !(info->mode & IP_TUNNEL_INFO_TX))) {
netdev_dbg(dev, "no tunnel metadata\n");
--
1.9.1
^ permalink raw reply related
* [PATCH v2 net 1/2] vxlan: avoid using stale vxlan socket.
From: Pravin B Shelar @ 2016-10-28 16:59 UTC (permalink / raw)
To: netdev; +Cc: Pravin B Shelar
When vxlan device is closed vxlan socket is freed. This
operation can race with vxlan-xmit function which
dereferences vxlan socket. Following patch uses RCU
mechanism to avoid this situation.
Signed-off-by: Pravin B Shelar <pshelar@ovn.org>
---
drivers/net/vxlan.c | 80 +++++++++++++++++++++++++++++++++--------------------
include/net/vxlan.h | 4 +--
2 files changed, 52 insertions(+), 32 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index c1639a3..f3c2fa3 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -943,17 +943,20 @@ static bool vxlan_snoop(struct net_device *dev,
static bool vxlan_group_used(struct vxlan_net *vn, struct vxlan_dev *dev)
{
struct vxlan_dev *vxlan;
+ struct vxlan_sock *sock4;
+ struct vxlan_sock *sock6 = NULL;
unsigned short family = dev->default_dst.remote_ip.sa.sa_family;
+ sock4 = rtnl_dereference(dev->vn4_sock);
+
/* The vxlan_sock is only used by dev, leaving group has
* no effect on other vxlan devices.
*/
- if (family == AF_INET && dev->vn4_sock &&
- atomic_read(&dev->vn4_sock->refcnt) == 1)
+ if (family == AF_INET && sock4 && atomic_read(&sock4->refcnt) == 1)
return false;
#if IS_ENABLED(CONFIG_IPV6)
- if (family == AF_INET6 && dev->vn6_sock &&
- atomic_read(&dev->vn6_sock->refcnt) == 1)
+ sock6 = rtnl_dereference(dev->vn6_sock);
+ if (family == AF_INET6 && sock6 && atomic_read(&sock6->refcnt) == 1)
return false;
#endif
@@ -961,10 +964,12 @@ static bool vxlan_group_used(struct vxlan_net *vn, struct vxlan_dev *dev)
if (!netif_running(vxlan->dev) || vxlan == dev)
continue;
- if (family == AF_INET && vxlan->vn4_sock != dev->vn4_sock)
+ if (family == AF_INET &&
+ rtnl_dereference(vxlan->vn4_sock) != sock4)
continue;
#if IS_ENABLED(CONFIG_IPV6)
- if (family == AF_INET6 && vxlan->vn6_sock != dev->vn6_sock)
+ if (family == AF_INET6 &&
+ rtnl_dereference(vxlan->vn6_sock) != sock6)
continue;
#endif
@@ -1005,22 +1010,25 @@ static bool __vxlan_sock_release_prep(struct vxlan_sock *vs)
static void vxlan_sock_release(struct vxlan_dev *vxlan)
{
- bool ipv4 = __vxlan_sock_release_prep(vxlan->vn4_sock);
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
#if IS_ENABLED(CONFIG_IPV6)
- bool ipv6 = __vxlan_sock_release_prep(vxlan->vn6_sock);
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ rcu_assign_pointer(vxlan->vn6_sock, NULL);
#endif
+ rcu_assign_pointer(vxlan->vn4_sock, NULL);
synchronize_net();
- if (ipv4) {
- udp_tunnel_sock_release(vxlan->vn4_sock->sock);
- kfree(vxlan->vn4_sock);
+ if (__vxlan_sock_release_prep(sock4)) {
+ udp_tunnel_sock_release(sock4->sock);
+ kfree(sock4);
}
#if IS_ENABLED(CONFIG_IPV6)
- if (ipv6) {
- udp_tunnel_sock_release(vxlan->vn6_sock->sock);
- kfree(vxlan->vn6_sock);
+ if (__vxlan_sock_release_prep(sock6)) {
+ udp_tunnel_sock_release(sock6->sock);
+ kfree(sock6);
}
#endif
}
@@ -1036,18 +1044,21 @@ static int vxlan_igmp_join(struct vxlan_dev *vxlan)
int ret = -EINVAL;
if (ip->sa.sa_family == AF_INET) {
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = ip->sin.sin_addr.s_addr,
.imr_ifindex = ifindex,
};
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
lock_sock(sk);
ret = ip_mc_join_group(sk, &mreq);
release_sock(sk);
#if IS_ENABLED(CONFIG_IPV6)
} else {
- sk = vxlan->vn6_sock->sock->sk;
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ sk = sock6->sock->sk;
lock_sock(sk);
ret = ipv6_stub->ipv6_sock_mc_join(sk, ifindex,
&ip->sin6.sin6_addr);
@@ -1067,18 +1078,21 @@ static int vxlan_igmp_leave(struct vxlan_dev *vxlan)
int ret = -EINVAL;
if (ip->sa.sa_family == AF_INET) {
+ struct vxlan_sock *sock4 = rtnl_dereference(vxlan->vn4_sock);
struct ip_mreqn mreq = {
.imr_multiaddr.s_addr = ip->sin.sin_addr.s_addr,
.imr_ifindex = ifindex,
};
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
lock_sock(sk);
ret = ip_mc_leave_group(sk, &mreq);
release_sock(sk);
#if IS_ENABLED(CONFIG_IPV6)
} else {
- sk = vxlan->vn6_sock->sock->sk;
+ struct vxlan_sock *sock6 = rtnl_dereference(vxlan->vn6_sock);
+
+ sk = sock6->sock->sk;
lock_sock(sk);
ret = ipv6_stub->ipv6_sock_mc_drop(sk, ifindex,
&ip->sin6.sin6_addr);
@@ -1828,11 +1842,15 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
struct dst_cache *dst_cache,
const struct ip_tunnel_info *info)
{
+ struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
bool use_cache = ip_tunnel_dst_cache_usable(skb, info);
struct dst_entry *ndst;
struct flowi6 fl6;
int err;
+ if (!sock6)
+ return ERR_PTR(-EIO);
+
if (tos && !info)
use_cache = false;
if (use_cache) {
@@ -1850,7 +1868,7 @@ static struct dst_entry *vxlan6_get_route(struct vxlan_dev *vxlan,
fl6.flowi6_proto = IPPROTO_UDP;
err = ipv6_stub->ipv6_dst_lookup(vxlan->net,
- vxlan->vn6_sock->sock->sk,
+ sock6->sock->sk,
&ndst, &fl6);
if (err < 0)
return ERR_PTR(err);
@@ -1995,9 +2013,11 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
}
if (dst->sa.sa_family == AF_INET) {
- if (!vxlan->vn4_sock)
+ struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
+
+ if (!sock4)
goto drop;
- sk = vxlan->vn4_sock->sock->sk;
+ sk = sock4->sock->sk;
rt = vxlan_get_route(vxlan, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2050,12 +2070,13 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
src_port, dst_port, xnet, !udp_sum);
#if IS_ENABLED(CONFIG_IPV6)
} else {
+ struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
struct dst_entry *ndst;
u32 rt6i_flags;
- if (!vxlan->vn6_sock)
+ if (!sock6)
goto drop;
- sk = vxlan->vn6_sock->sock->sk;
+ sk = sock6->sock->sk;
ndst = vxlan6_get_route(vxlan, skb,
rdst ? rdst->remote_ifindex : 0, tos,
@@ -2415,9 +2436,10 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
dport = info->key.tp_dst ? : vxlan->cfg.dst_port;
if (ip_tunnel_info_af(info) == AF_INET) {
+ struct vxlan_sock *sock4 = rcu_dereference(vxlan->vn4_sock);
struct rtable *rt;
- if (!vxlan->vn4_sock)
+ if (!sock4)
return -EINVAL;
rt = vxlan_get_route(vxlan, skb, 0, info->key.tos,
info->key.u.ipv4.dst,
@@ -2429,8 +2451,6 @@ static int vxlan_fill_metadata_dst(struct net_device *dev, struct sk_buff *skb)
#if IS_ENABLED(CONFIG_IPV6)
struct dst_entry *ndst;
- if (!vxlan->vn6_sock)
- return -EINVAL;
ndst = vxlan6_get_route(vxlan, skb, 0, info->key.tos,
info->key.label, &info->key.u.ipv6.dst,
&info->key.u.ipv6.src, NULL, info);
@@ -2740,10 +2760,10 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
return PTR_ERR(vs);
#if IS_ENABLED(CONFIG_IPV6)
if (ipv6)
- vxlan->vn6_sock = vs;
+ rcu_assign_pointer(vxlan->vn6_sock, vs);
else
#endif
- vxlan->vn4_sock = vs;
+ rcu_assign_pointer(vxlan->vn4_sock, vs);
vxlan_vs_add_dev(vs, vxlan);
return 0;
}
@@ -2754,9 +2774,9 @@ static int vxlan_sock_add(struct vxlan_dev *vxlan)
bool metadata = vxlan->flags & VXLAN_F_COLLECT_METADATA;
int ret = 0;
- vxlan->vn4_sock = NULL;
+ RCU_INIT_POINTER(vxlan->vn4_sock, NULL);
#if IS_ENABLED(CONFIG_IPV6)
- vxlan->vn6_sock = NULL;
+ RCU_INIT_POINTER(vxlan->vn6_sock, NULL);
if (ipv6 || metadata)
ret = __vxlan_sock_add(vxlan, true);
#endif
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 0255613..308adc4 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -225,9 +225,9 @@ struct vxlan_config {
struct vxlan_dev {
struct hlist_node hlist; /* vni hash table */
struct list_head next; /* vxlan's per namespace list */
- struct vxlan_sock *vn4_sock; /* listening socket for IPv4 */
+ struct vxlan_sock __rcu *vn4_sock; /* listening socket for IPv4 */
#if IS_ENABLED(CONFIG_IPV6)
- struct vxlan_sock *vn6_sock; /* listening socket for IPv6 */
+ struct vxlan_sock __rcu *vn6_sock; /* listening socket for IPv6 */
#endif
struct net_device *dev;
struct net *net; /* netns for packet i/o */
--
1.9.1
^ permalink raw reply related
* [PATCH net] dctcp: avoid bogus doubling of cwnd after loss
From: Florian Westphal @ 2016-10-28 16:43 UTC (permalink / raw)
To: netdev
Cc: daniel, Florian Westphal, Lawrence Brakmo, Andrew Shewmaker,
Glenn Judd
If a congestion control module doesn't provide .undo_cwnd function,
tcp_undo_cwnd_reduction() will set cwnd to
tp->snd_cwnd = max(tp->snd_cwnd, tp->snd_ssthresh << 1);
... which makes sense for reno (it sets ssthresh to half the current cwnd),
but it makes no sense for dctcp, which sets ssthresh based on the current
congestion estimate.
This can cause severe growth of cwnd (eventually overflowing u32).
Fix this by saving last cwnd on loss and restore cwnd based on that,
similar to cubic and other algorithms.
Fixes: e3118e8359bb7c ("net: tcp: add DCTCP congestion control algorithm")
Cc: Lawrence Brakmo <brakmo@fb.com>
Cc: Andrew Shewmaker <agshew@gmail.com>
Cc: Glenn Judd <glenn.judd@morganstanley.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/ipv4/tcp_dctcp.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index 10d728b6804c..ab37c6775630 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -56,6 +56,7 @@ struct dctcp {
u32 next_seq;
u32 ce_state;
u32 delayed_ack_reserved;
+ u32 loss_cwnd;
};
static unsigned int dctcp_shift_g __read_mostly = 4; /* g = 1/2^4 */
@@ -96,6 +97,7 @@ static void dctcp_init(struct sock *sk)
ca->dctcp_alpha = min(dctcp_alpha_on_init, DCTCP_MAX_ALPHA);
ca->delayed_ack_reserved = 0;
+ ca->loss_cwnd = 0;
ca->ce_state = 0;
dctcp_reset(tp, ca);
@@ -111,9 +113,10 @@ static void dctcp_init(struct sock *sk)
static u32 dctcp_ssthresh(struct sock *sk)
{
- const struct dctcp *ca = inet_csk_ca(sk);
+ struct dctcp *ca = inet_csk_ca(sk);
struct tcp_sock *tp = tcp_sk(sk);
+ ca->loss_cwnd = tp->snd_cwnd;
return max(tp->snd_cwnd - ((tp->snd_cwnd * ca->dctcp_alpha) >> 11U), 2U);
}
@@ -308,12 +311,20 @@ static size_t dctcp_get_info(struct sock *sk, u32 ext, int *attr,
return 0;
}
+static u32 dctcp_cwnd_undo(struct sock *sk)
+{
+ const struct dctcp *ca = inet_csk_ca(sk);
+
+ return max(tcp_sk(sk)->snd_cwnd, ca->loss_cwnd);
+}
+
static struct tcp_congestion_ops dctcp __read_mostly = {
.init = dctcp_init,
.in_ack_event = dctcp_update_alpha,
.cwnd_event = dctcp_cwnd_event,
.ssthresh = dctcp_ssthresh,
.cong_avoid = tcp_reno_cong_avoid,
+ .undo_cwnd = dctcp_cwnd_undo,
.set_state = dctcp_state,
.get_info = dctcp_get_info,
.flags = TCP_CONG_NEEDS_ECN,
--
2.7.3
^ permalink raw reply related
* Re: Bonding MAC address
From: Shanker Wang @ 2016-10-28 16:39 UTC (permalink / raw)
To: Igor Ryzhov; +Cc: netdev
In-Reply-To: <64717203-D02C-41D0-9D2B-EFEB4CF78146@nfware.com>
[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]
I think it is intentional. Because changing mac address of an
interface would cause problems. As a result, if you find it
necessary to change the mac address, it should be done manually.
Consider these two example:
* A host is connecting the internet through a bond interface,
and obtain ip address from a dhcp server. Changing the mac
address automatically would possibly lead to re-assigning a
new IP address, which could not be expected.
* A linux-box is acting as a gateway, providing service to
hosts in the local lan. Changing mac address and not changing
IP address of an interface would cause other hosts to fail
to communicate with the gateway, since the old mac address is
still in theirs arp cache table. The communication will recover
after arp cache expires, which can be a short or long time.
* The scene is the same to the second one. And consider if
arp snooping or other mechanisms to protect hosts from being
spoofed by a fake gateway are enabled in the local lan. After
changing the mac address, the linux-box itself will be a
“spoofer” and may get blocked.
So changing mac address of an interface could be dangerous and
lead to network malfunction and cannot be done automatically.
> 在 2016年10月28日，19:09，Igor Ryzhov <iryzhov@nfware.com> 写道：
>
> Hello everyone,
>
> Studying the bonding driver I found that bonding device doesn't choose a new MAC address when the slave which MAC is in use is released.
> This is a warning about that - "the permanent HWaddr of slave is still in use by bond - set the HWaddr of slave to a different address to avoid conflicts".
> Why not to choose a new MAC for bonding device? Is it intentional or just not implemented?
>
> Best regards,
> Igor
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3599 bytes --]
^ permalink raw reply
* Re: [PATCH] Revert "usb: dwc3: gadget: use allocated/queued reqs for LST bit"
From: Ville Syrjälä @ 2016-10-28 16:33 UTC (permalink / raw)
To: Felipe Balbi
Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA, stable-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, Oliver Neukum,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87pomksrrm.fsf-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Fri, Oct 28, 2016 at 01:16:13PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
> > On Thu, Oct 06, 2016 at 12:08:26PM +0300, Ville Syrjälä wrote:
> >> On Thu, Oct 06, 2016 at 10:36:09AM +0300, Felipe Balbi wrote:
> >> >
> >> > Hi,
> >> >
> >> > Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:
> >> <snip>
> >> > Okay, I have found a regression on dwc3 and another patch follows:
> >> >
> >> > commit 5e1a2af3e46248c55098cdae643c4141851b703e
> >> > Author: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >> > Date: Wed Oct 5 14:24:37 2016 +0300
> >> >
> >> > usb: dwc3: gadget: properly account queued requests
> >> >
> >> > Some requests could be accounted for multiple
> >> > times. Let's fix that so each and every requests is
> >> > accounted for only once.
> >> >
> >> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> # v4.8
> >> > Fixes: 55a0237f8f47 ("usb: dwc3: gadget: use allocated/queued reqs for LST bit")
> >> > Signed-off-by: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> >> >
> >> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> >> > index 07cc8929f271..3c3ced128c77 100644
> >> > --- a/drivers/usb/dwc3/gadget.c
> >> > +++ b/drivers/usb/dwc3/gadget.c
> >> > @@ -783,6 +783,7 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> >> > req->trb = trb;
> >> > req->trb_dma = dwc3_trb_dma_offset(dep, trb);
> >> > req->first_trb_index = dep->trb_enqueue;
> >> > + dep->queued_requests++;
> >> > }
> >> >
> >> > dwc3_ep_inc_enq(dep);
> >> > @@ -833,8 +834,6 @@ static void dwc3_prepare_one_trb(struct dwc3_ep *dep,
> >> >
> >> > trb->ctrl |= DWC3_TRB_CTRL_HWO;
> >> >
> >> > - dep->queued_requests++;
> >> > -
> >> > trace_dwc3_prepare_trb(dep, trb);
> >> > }
> >> >
> >> > @@ -1861,8 +1860,11 @@ static int __dwc3_cleanup_done_trbs(struct dwc3 *dwc, struct dwc3_ep *dep,
> >> > unsigned int s_pkt = 0;
> >> > unsigned int trb_status;
> >> >
> >> > - dep->queued_requests--;
> >> > dwc3_ep_inc_deq(dep);
> >> > +
> >> > + if (req->trb == trb)
> >> > + dep->queued_requests--;
> >> > +
> >> > trace_dwc3_complete_trb(dep, trb);
> >> >
> >> > /*
> >> >
> >> > I have also built a branch which you can use for testing. Here's a pull
> >> > request, once you tell me it works for you, then I can send proper
> >> > patches out:
> >> >
> >> > The following changes since commit c8d2bc9bc39ebea8437fd974fdbc21847bb897a3:
> >> >
> >> > Linux 4.8 (2016-10-02 16:24:33 -0700)
> >> >
> >> > are available in the git repository at:
> >> >
> >> > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git tmp-test-v4.8
> >> >
> >> > for you to fetch changes up to c968b8d1effe64a7980802d1eef29f4e1922faca:
> >> >
> >> > usb: dwc3: gadget: properly account queued requests (2016-10-06 10:16:37 +0300)
> >> >
> >> > ----------------------------------------------------------------
> >> > Felipe Balbi (2):
> >> > usb: gadget: function: u_ether: don't starve tx request queue
> >> > usb: dwc3: gadget: properly account queued requests
> >> >
> >> > drivers/usb/dwc3/gadget.c | 7 ++++---
> >> > drivers/usb/gadget/function/u_ether.c | 5 +++--
> >> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >>
> >> Tried your branch, but unfortunately I'm still seeing the lags. New trace
> >> attached.
> >
> > Just a reminder that the regressions is still there in 4.9-rc2.
> > Unfortunateyly with all the stuff already piled on top, getting USB
> > working on my device is no longer a simple matter of reverting the
> > one commit. I had to revert 10 patches to get even a clean revert, but
> > unfortunately that approach just lead to the transfer hanging immediately.
> >
> > So what I ended up doing is reverting all of this:
> > git log --no-merges --format=oneline 55a0237f8f47957163125e20ee9260538c5c341c^..HEAD -- drivers/usb/dwc3/ include/linux/ulpi/interface.h drivers/usb/Kconfig drivers/usb/core/Kconfig
> >
> > which comes out at whopping 70 commits. Having to carry that around
> > is going to be quite a pain especially as more stuff might be piled on
> > top.
> >
> > /me thinks a stable backport of any fix (assuming one is found
> > eventually) is going to be quite the challenge...
>
> Yeah, I'm guessing we're gonna need some help from networking folks. The
> only thing we did since v4.7 was actually respect req->no_interrupt flag
> coming from u_ether itself. No idea why that causes so much trouble for
> u_ether.
>
> BTW, Instead of reverting so many patches, you can just remove
> throttling:
>
> diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
> index f4a640216913..119a2e5848e8 100644
> --- a/drivers/usb/gadget/function/u_ether.c
> +++ b/drivers/usb/gadget/function/u_ether.c
> @@ -589,14 +589,6 @@ static netdev_tx_t eth_start_xmit(struct sk_buff *skb,
>
> req->length = length;
>
> - /* throttle high/super speed IRQ rate back slightly */
> - if (gadget_is_dualspeed(dev->gadget))
> - req->no_interrupt = (((dev->gadget->speed == USB_SPEED_HIGH ||
> - dev->gadget->speed == USB_SPEED_SUPER)) &&
> - !list_empty(&dev->tx_reqs))
> - ? ((atomic_read(&dev->tx_qlen) % dev->qmult) != 0)
> - : 0;
> -
> retval = usb_ep_queue(in, req, GFP_ATOMIC);
> switch (retval) {
> default:
Ah cool. That indeed fixes the problem for me.
>
> I'm adding netdev and couple other folks to the loop.
>
> Just to summarize, USB peripheral controller now actually throttles
> interrupt when requested to do so and that causes lags for USB
> networking gadgets.
>
> Without throttle we, potentially, call netif_wake_queue() more
> frequently than with throttling. I'm wondering if something changed in
> NET layer within the past few years but the USB networking gadgets ended
> up being forgotten.
>
> Anyway, if anybody has any hints, I'd be glad to hear about them.
>
> --
> balbi
--
Ville Syrjälä
Intel OTC
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" 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
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
From: Florian Westphal @ 2016-10-28 16:21 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Florian Westphal, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <18995804.9DSMHNUO14@wuerfel>
Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, October 28, 2016 5:50:31 PM CEST Florian Westphal wrote:
> > Arnd Bergmann <arnd@arndb.de> wrote:
> > > The newly added nft fib code produces two warnings:
> > >
> > > net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> > > net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> > > net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> > > net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > >
> > > The first one is obvious as the only user of that variable is
> > > inside of an #ifdef, but the second one is a bit trickier.
> > > It is clear that 'oif' is uninitialized here if neither
> > > NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
> > >
> > > I have no idea how that should be handled, this patch just
> > > returns without doing anything, which may or may not be
> > > the right thing to do.
> >
> > It should be initialized to NULL.
>
> Ok, I had considered that, but wasn't sure if ->nh_dev could
> ever be NULL, as that would then get dereferenced.
Good point. In case oif is NULL we don't have to search the result
list for a match anyway, so we could do this (not even build tested):
diff --git a/net/ipv4/netfilter/nft_fib_ipv4.c b/net/ipv4/netfilter/nft_fib_ipv4.c
--- a/net/ipv4/netfilter/nft_fib_ipv4.c
+++ b/net/ipv4/netfilter/nft_fib_ipv4.c
@@ -130,6 +130,11 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
break;
}
+ if (!oif) {
+ found = FIB_RES_DEV(res);
+ goto ok;
+ }
+
#ifdef CONFIG_IP_ROUTE_MULTIPATH
for (i = 0; i < res.fi->fib_nhs; i++) {
struct fib_nh *nh = &res.fi->fib_nh[i];
@@ -139,16 +144,12 @@ void nft_fib4_eval(const struct nft_expr *expr, struct nft_regs *regs,
goto ok;
}
}
-#endif
- if (priv->flags & NFTA_FIB_F_OIF) {
- found = FIB_RES_DEV(res);
- if (found == oif)
- goto ok;
- return;
- }
-
- *dest = FIB_RES_DEV(res)->ifindex;
return;
+#else
+ found = FIB_RES_DEV(res);
+ if (found != oif)
+ return;
+#endif
ok:
switch (priv->result) {
I can take care of this as a followup.
^ permalink raw reply
* Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: Jakub Kicinski @ 2016-10-28 16:18 UTC (permalink / raw)
To: John Fastabend
Cc: David Miller, alexander.duyck, mst, brouer, shrijeet, tom, netdev,
shm, roopa, nikolay
In-Reply-To: <58137533.4030105@gmail.com>
On Fri, 28 Oct 2016 08:56:35 -0700, John Fastabend wrote:
> On 16-10-27 07:10 PM, David Miller wrote:
> > From: Alexander Duyck <alexander.duyck@gmail.com>
> > Date: Thu, 27 Oct 2016 18:43:59 -0700
> >
> >> On Thu, Oct 27, 2016 at 6:35 PM, David Miller <davem@davemloft.net> wrote:
> >>> From: "Michael S. Tsirkin" <mst@redhat.com>
> >>> Date: Fri, 28 Oct 2016 01:25:48 +0300
> >>>
> >>>> On Thu, Oct 27, 2016 at 05:42:18PM -0400, David Miller wrote:
> >>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
> >>>>> Date: Fri, 28 Oct 2016 00:30:35 +0300
> >>>>>
> >>>>>> Something I'd like to understand is how does XDP address the
> >>>>>> problem that 100Byte packets are consuming 4K of memory now.
> >>>>>
> >>>>> Via page pools. We're going to make a generic one, but right now
> >>>>> each and every driver implements a quick list of pages to allocate
> >>>>> from (and thus avoid the DMA man/unmap overhead, etc.)
> >>>>
> >>>> So to clarify, ATM virtio doesn't attempt to avoid dma map/unmap
> >>>> so there should be no issue with that even when using sub/page
> >>>> regions, assuming DMA APIs support sub-page map/unmap correctly.
> >>>
> >>> That's not what I said.
> >>>
> >>> The page pools are meant to address the performance degradation from
> >>> going to having one packet per page for the sake of XDP's
> >>> requirements.
> >>>
> >>> You still need to have one packet per page for correct XDP operation
> >>> whether you do page pools or not, and whether you have DMA mapping
> >>> (or it's equivalent virutalization operation) or not.
> >>
> >> Maybe I am missing something here, but why do you need to limit things
> >> to one packet per page for correct XDP operation? Most of the drivers
> >> out there now are usually storing something closer to at least 2
> >> packets per page, and with the DMA API fixes I am working on there
> >> should be no issue with changing the contents inside those pages since
> >> we won't invalidate or overwrite the data after the DMA buffer has
> >> been synchronized for use by the CPU.
> >
> > Because with SKB's you can share the page with other packets.
> >
> > With XDP you simply cannot.
> >
> > It's software semantics that are the issue. SKB frag list pages
> > are read only, XDP packets are writable.
> >
> > This has nothing to do with "writability" of the pages wrt. DMA
> > mapping or cpu mappings.
> >
>
> Sorry I'm not seeing it either. The current xdp_buff is defined
> by,
>
> struct xdp_buff {
> void *data;
> void *data_end;
> };
>
> The verifier has an xdp_is_valid_access() check to ensure we don't go
> past data_end. The page for now at least never leaves the driver. For
> the work to get xmit to other devices working I'm still not sure I see
> any issue.
+1
Do we want to make the packet-per-page a requirement because it could
be useful in the future from architectural standpoint? I guess there
is a trade-off here between having the comfort of people following this
requirement today and making driver support for XDP even more complex.
^ permalink raw reply
* Re: [PATCH] [rfc, netfilter-next] netfilter: nf_tables: fib warnings
From: Arnd Bergmann @ 2016-10-28 16:15 UTC (permalink / raw)
To: Florian Westphal
Cc: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, netfilter-devel, coreteam, netdev,
linux-kernel
In-Reply-To: <20161028155031.GI23011@breakpoint.cc>
On Friday, October 28, 2016 5:50:31 PM CEST Florian Westphal wrote:
> Arnd Bergmann <arnd@arndb.de> wrote:
> > The newly added nft fib code produces two warnings:
> >
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function 'nft_fib4_eval':
> > net/ipv4/netfilter/nft_fib_ipv4.c:80:6: error: unused variable 'i' [-Werror=unused-variable]
> > net/ipv4/netfilter/nft_fib_ipv4.c: In function ‘nft_fib4_eval’:
> > net/ipv4/netfilter/nft_fib_ipv4.c:137:6: error: ‘oif’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > The first one is obvious as the only user of that variable is
> > inside of an #ifdef, but the second one is a bit trickier.
> > It is clear that 'oif' is uninitialized here if neither
> > NFTA_FIB_F_OIF nor NFTA_FIB_F_IIF are set.
> >
> > I have no idea how that should be handled, this patch just
> > returns without doing anything, which may or may not be
> > the right thing to do.
>
> It should be initialized to NULL.
Ok, I had considered that, but wasn't sure if ->nh_dev could
ever be NULL, as that would then get dereferenced.
Arnd
^ permalink raw reply
* Re: [PATCH v2] ip6_tunnel: Clear IP6CB in ip6_tnl_xmit() after encapsulation
From: Shmulik Ladkani @ 2016-10-28 16:07 UTC (permalink / raw)
To: Eli Cooper; +Cc: Tom Herbert, Linux Kernel Network Developers, David S . Miller
In-Reply-To: <9a1bb72b-246d-021a-236c-f523832964ac@gmx.com>
Hi,
On Fri, 28 Oct 2016 13:13:45 +0800 Eli Cooper <elicooper@gmx.com> wrote:
> So I think it is best that all the
> IP6CB gets cleared before it is pushed to the next layer.
Just a comparison to the ipv4 world:
All tunnels (udp/ip based) end up calling iptunnel_xmit(), which:
- scrubs the skb
- clears any IPCB residues
- installs the iphdr
- invokes ip_local_out()
OTOH ip6_tnl_xmit:
- scrubs the skb
- installs the ipv6hdr
- invokes ip6tunnel_xmit() - a thin wrapper to ip6_local_out()
* missing: clearing cb
And OTOH udp_tunnel6_xmit_skb:
- clears IPCB(skb)->opt and some IPCB(skb)->flags
(why these 2 explicitly? and why at this point? and IPCB is no longer
relevant...)
- installs the ipv6hdr
- invokes ip6tunnel_xmit() - a thin wrapper to ip6_local_out()
* missing: scrub, clearing cb
> Maybe we
> should clear IP6CB in ip6tunnel_xmit(), rather than in every tunnel's codes?
This seems reasonable.
A potential issue might be whether it needs to be done earlier, although
I've reviewed current versions of both 'ip6_tnl_xmit' and
'udp_tunnel6_xmit_skb' and it looks okay. But please verify.
> By the way, I don't see any point in setting IPCB(skb)->flags in
> udp_tunnel6_xmit_skb(). It will not be interpreted as IPCB any further
> past ip6tunnel_xmit(), even if it were not cleared. Plus, nothing seems
> to use these flags anyway.
This seems right.
It was introduced in 6a93cc9052 "udp-tunnel: Add a few more UDP tunnel APIs".
If you checkout that tree, you'll notice same treatment to
IPCB(skb)->opt and IPCB(skb)->flags in l2tp_xmit_skb... maybe it was
just copied ;-)
Best,
Shmulik
^ permalink raw reply
* Re: [PATCH net-next RFC WIP] Patch for XDP support for virtio_net
From: John Fastabend @ 2016-10-28 15:56 UTC (permalink / raw)
To: David Miller, alexander.duyck
Cc: mst, brouer, shrijeet, tom, netdev, shm, roopa, nikolay
In-Reply-To: <20161027.221027.109834362557507518.davem@davemloft.net>
On 16-10-27 07:10 PM, David Miller wrote:
> From: Alexander Duyck <alexander.duyck@gmail.com>
> Date: Thu, 27 Oct 2016 18:43:59 -0700
>
>> On Thu, Oct 27, 2016 at 6:35 PM, David Miller <davem@davemloft.net> wrote:
>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>> Date: Fri, 28 Oct 2016 01:25:48 +0300
>>>
>>>> On Thu, Oct 27, 2016 at 05:42:18PM -0400, David Miller wrote:
>>>>> From: "Michael S. Tsirkin" <mst@redhat.com>
>>>>> Date: Fri, 28 Oct 2016 00:30:35 +0300
>>>>>
>>>>>> Something I'd like to understand is how does XDP address the
>>>>>> problem that 100Byte packets are consuming 4K of memory now.
>>>>>
>>>>> Via page pools. We're going to make a generic one, but right now
>>>>> each and every driver implements a quick list of pages to allocate
>>>>> from (and thus avoid the DMA man/unmap overhead, etc.)
>>>>
>>>> So to clarify, ATM virtio doesn't attempt to avoid dma map/unmap
>>>> so there should be no issue with that even when using sub/page
>>>> regions, assuming DMA APIs support sub-page map/unmap correctly.
>>>
>>> That's not what I said.
>>>
>>> The page pools are meant to address the performance degradation from
>>> going to having one packet per page for the sake of XDP's
>>> requirements.
>>>
>>> You still need to have one packet per page for correct XDP operation
>>> whether you do page pools or not, and whether you have DMA mapping
>>> (or it's equivalent virutalization operation) or not.
>>
>> Maybe I am missing something here, but why do you need to limit things
>> to one packet per page for correct XDP operation? Most of the drivers
>> out there now are usually storing something closer to at least 2
>> packets per page, and with the DMA API fixes I am working on there
>> should be no issue with changing the contents inside those pages since
>> we won't invalidate or overwrite the data after the DMA buffer has
>> been synchronized for use by the CPU.
>
> Because with SKB's you can share the page with other packets.
>
> With XDP you simply cannot.
>
> It's software semantics that are the issue. SKB frag list pages
> are read only, XDP packets are writable.
>
> This has nothing to do with "writability" of the pages wrt. DMA
> mapping or cpu mappings.
>
Sorry I'm not seeing it either. The current xdp_buff is defined
by,
struct xdp_buff {
void *data;
void *data_end;
};
The verifier has an xdp_is_valid_access() check to ensure we don't go
past data_end. The page for now at least never leaves the driver. For
the work to get xmit to other devices working I'm still not sure I see
any issue.
.John
^ permalink raw reply
* Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517
From: Tony Lindgren @ 2016-10-28 15:52 UTC (permalink / raw)
To: Jeroen Hofstee
Cc: netdev, Mugunthan V N, Grygorii Strashko,
open list:TI ETHERNET SWITCH DRIVER (CPSW), open list
In-Reply-To: <1477668756-2651-1-git-send-email-jhofstee@victronenergy.com>
* Jeroen Hofstee <jhofstee@victronenergy.com> [161028 08:33]:
> Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
> id to common file") did not only move the code for an am3517, it also
> added the slave parameter, resulting in an invalid (all zero) mac address
> being returned for an am3517, since it only has a single emac and the slave
> parameter is pointing to the second. So simply always read the first and
> valid mac-address for a ti,am3517-emac.
And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?
Regards,
Tony
^ 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