* Re: [PATCH v4 3/4] ARM: shmobile: r7s72100: Add clock for r7s72100-ether
From: Sergei Shtylyov @ 2014-01-08 21:03 UTC (permalink / raw)
To: Simon Horman, David S. Miller, netdev, linux-sh
Cc: linux-arm-kernel, Magnus Damm
In-Reply-To: <1389168152-9434-4-git-send-email-horms+renesas@verge.net.au>
Hello.
On 01/08/2014 11:02 AM, Simon Horman wrote:
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net-next 2/3] virtio-net: use per-receive queue page frag alloc for mergeable bufs
From: Rick Jones @ 2014-01-08 21:16 UTC (permalink / raw)
To: Eric Dumazet, Michael S. Tsirkin
Cc: Michael Dalton, netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <1389204587.26646.111.camel@edumazet-glaptop2.roam.corp.google.com>
On 01/08/2014 10:09 AM, Eric Dumazet wrote:
> In normal networking land, when a host A sends frames to host B,
> nothing prevents A to pause the traffic to B if B is dropping packets
> under stress.
>
> A physical NIC do not use a workqueue to refill its RX queue but uses
> the following strategy :
>
> 0) Pre filling of RX ring buffer with N frames. This can use GFP_KERNEL
> allocations with all needed (sleep/retry/shout) logic...
> 1) IRQ is handled.
> 2) Can we allocate a new buffer (GFP_ATOMIC) ?
> If yes, we accept the frame,
> and post the new buffer for the 'next frame'
> If no, we drop the frame and recycle the memory for next round.
and increment a suitably specific statistic so someone trying
to diagnose performance/other problems can know we dropped the frame.
rick jones
^ permalink raw reply
* Re: [PATCH] unix: show socket peer if no addr is given in /proc/net/unix
From: Sergei Shtylyov @ 2014-01-08 22:19 UTC (permalink / raw)
To: Masatake YAMATO, netdev
In-Reply-To: <1389158288-2855-1-git-send-email-yamato@redhat.com>
Hello.
On 01/08/2014 08:18 AM, Masatake YAMATO wrote:
> Path field of /proc/net/unix is empty if an address is not given
> to a socket. Typical way to create such socket is calling
> socketpair. The empty fields make it difficult to understand the
> communication between processes. e.g. lsof cannot resolve the role of
> file descriptors well.
> This patch fills the empty fields with unix_peer.
> Signed-off-by: Masatake YAMATO <yamato@redhat.com>
> ---
> net/unix/af_unix.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 800ca61..1700133 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2340,7 +2340,9 @@ static int unix_seq_show(struct seq_file *seq, void *v)
> else {
> struct sock *s = v;
> struct unix_sock *u = unix_sk(s);
> + struct sock *s_peer;
> unix_state_lock(s);
> + s_peer = unix_peer(s);
>
> seq_printf(seq, "%pK: %08X %08X %08X %04X %02X %5lu",
> s,
> @@ -2367,7 +2369,8 @@ static int unix_seq_show(struct seq_file *seq, void *v)
> }
> for ( ; i < len; i++)
> seq_putc(seq, u->addr->name->sun_path[i]);
> - }
> + } else if (s_peer)
> + seq_printf(seq, " #%pK", s_peer);
According to Documentation/CodingStyle, both arms of the *if* statement
should have {} if one has it.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net] xen-netback: fix vif tx queue race in xenvif_rx_interrupt
From: Zoltan Kiss @ 2014-01-08 21:18 UTC (permalink / raw)
To: Ma JieYue, netdev, xen-devel
Cc: Ma JieYue, Wang Yingbin, Fu Tienan, Wei Liu, Ian Campbell,
David Vrabel
In-Reply-To: <1389209061-29494-1-git-send-email-jieyue.majy@alibaba-inc.com>
Hi,
With Paul's recent flow control improvement I think this became invalid:
http://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/commit/?id=ca2f09f2b2c6c25047cfc545d057c4edfcfe561c
Zoli
On 08/01/14 19:24, Ma JieYue wrote:
> From: Ma JieYue <jieyue.majy@alibaba-inc.com>
>
> There is a race when waking up or stopping xenvif tx queue, and it leads to
> unnecessary packet drop. The problem is that the rx ring still full when entering
> into xenvif_start_xmit. In fact, in xenvif_rx_interrupt, the netif_wake_queue
> may be called not just after the ring is not full any more, so the operation
> is not atomic. Here is part of the debug log when the race scenario happened:
>
> wake_queue: req_cons_peek 2679757 req_cons 2679586 req_prod 2679841
> stop_queue: req_cons_peek 2679837 req_cons 2679757 req_prod 2679841
> [tx_queue_stopped true]
> wake_queue: req_cons_peek 2679837 req_cons 2679757 req_prod 2679841
> [tx_queue_stopped false]
> drop packet: req_cons_peek 2679837 req_cons 2679757 req_prod 2679841
>
> The debug log was written, every time right after netif_wake_queue been called
> in xenvif_rx_interrupt, every time after netif_stop_queue been called in
> xenvif_start_xmit and every time packet drop happened in xenvif_start_xmit.
> As we can see, the second wake_queue appeared in the place it should not be, and
> we believed the ring had been checked before the stop_queue, but the actual
> wake_queue action didn't follow, and took place after the stop_queue, so that when
> entering into xenvif_start_xmit the ring was full but the queue was not stopped.
>
> The patch fixes the race by checking if tx queue stopped, before trying to
> wake it up in xenvif_rx_interrupt. It only wakes the queue when it is stopped,
> as well as it is not full and schedulable.
>
> Signed-off-by: Ma JieYue <jieyue.majy@alibaba-inc.com>
> Signed-off-by: Wang Yingbin <yingbin.wangyb@alibaba-inc.com>
> Signed-off-by: Fu Tienan <tienan.ftn@alibaba-inc.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/net/xen-netback/interface.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index fff8cdd..e099f62 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -105,7 +105,7 @@ static irqreturn_t xenvif_rx_interrupt(int irq, void *dev_id)
> {
> struct xenvif *vif = dev_id;
>
> - if (xenvif_rx_schedulable(vif))
> + if (netif_queue_stopped(vif->dev) && xenvif_rx_schedulable(vif))
> netif_wake_queue(vif->dev);
>
> return IRQ_HANDLED;
>
^ permalink raw reply
* Re: [PATCH net-next v3 8/9] xen-netback: Timeout packets in RX path
From: Zoltan Kiss @ 2014-01-08 21:34 UTC (permalink / raw)
To: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
Cc: Zoltan Kiss, Paul Durrant
In-Reply-To: <1389139818-24458-9-git-send-email-zoltan.kiss@citrix.com>
I just realized when answering Ma's mail that this doesn't cause the
desired effect after Paul's flow control improvement: starting the queue
doesn't drop the packets which cannot fit the ring. Which in fact might
be not good. We are adding the skb to vif->rx_queue even when
xenvif_rx_ring_slots_available(vif, min_slots_needed) said there is no
space for that. Or am I missing something? Paul?
Zoli
On 08/01/14 00:10, Zoltan Kiss wrote:
> A malicious or buggy guest can leave its queue filled indefinitely, in which
> case qdisc start to queue packets for that VIF. If those packets came from an
> another guest, it can block its slots and prevent shutdown. To avoid that, we
> make sure the queue is drained in every 10 seconds.
...
> diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
> index 95fcd63..ce032f9 100644
> --- a/drivers/net/xen-netback/interface.c
> +++ b/drivers/net/xen-netback/interface.c
> @@ -114,6 +114,16 @@ static irqreturn_t xenvif_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static void xenvif_wake_queue(unsigned long data)
> +{
> + struct xenvif *vif = (struct xenvif *)data;
> +
> + if (netif_queue_stopped(vif->dev)) {
> + netdev_err(vif->dev, "draining TX queue\n");
> + netif_wake_queue(vif->dev);
> + }
> +}
> +
> static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct xenvif *vif = netdev_priv(dev);
> @@ -143,8 +153,13 @@ static int xenvif_start_xmit(struct sk_buff *skb, struct net_device *dev)
> * then turn off the queue to give the ring a chance to
> * drain.
> */
> - if (!xenvif_rx_ring_slots_available(vif, min_slots_needed))
> + if (!xenvif_rx_ring_slots_available(vif, min_slots_needed)) {
> + vif->wake_queue.function = xenvif_wake_queue;
> + vif->wake_queue.data = (unsigned long)vif;
> xenvif_stop_queue(vif);
> + mod_timer(&vif->wake_queue,
> + jiffies + rx_drain_timeout_jiffies);
> + }
>
> skb_queue_tail(&vif->rx_queue, skb);
> xenvif_kick_thread(vif);
^ permalink raw reply
* I have a very lucrative Business
From: Dr. Musa Hamed @ 2014-01-08 21:09 UTC (permalink / raw)
ATTN: Dear,
I am Dr. Musa Hamed Executive Director of the Bank of Africa
Plc B.O.A,Cotonou Benin Rep. I have a very lucrative Business proposal
worth of 21,500,000.00 USD.
An Iraqi named Hamadi Hashem a business man made a fixed deposit of
21,500,000.00 USD . Upon maturity several notices was sent to him,
even during the war,eleven years ago (2003) up till this date. Again
after the war another notification was sent and still no response came
from him. it has been found out that Hamadi Hashem and his family had
been killed during the war in bomb blast that hit their home at
Mukaradeeb where his personal oil well was. I am prepared to split the
funds with you 50%:50%.By placing you as the NEXT OF KIN to his
deposit.
Should you be interested in doing this deal with me, please send me
the following information:
(1) Your Name
(2) Resident Address
(3) Your Occupation
(4) Your Phone Number
(5) Date of Birth
(6) Resident Country
And I will prefer you to reach me privately and securely on my personal
email address: dr.musahamed30@yahoo.fr
Tel: +229-988-400-97
I will provide you with further information as soon as I hear
positively from you.
Regards,
Dr. Musa Hamed
^ permalink raw reply
* [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option
From: Ben Hutchings @ 2014-01-08 21:52 UTC (permalink / raw)
To: Or Gerlitz, Bill Fink, netdev
Before the generic features API was introduced, the ethtool -K option
took short names for various features, e.g. 'gso' but the -k option
reported their state using longer names,
e.g. 'generic-segmentation-offload'.
All newer features have a single kernel-provided name so input
and output are consistent. But the old features still aren't, and
although their short names are documented it's not good to have
these exceptions.
Change the argument parsing code for -K so that the long names
reported by -k are also accepted.
Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
This turned out to be pretty easy to do as the argument processing is
all table-driven already. I'll push this if it works for you.
Ben.
ethtool.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/ethtool.c b/ethtool.c
index b06dfa3..4226d2e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2053,25 +2053,31 @@ static int do_sfeatures(struct cmd_context *ctx)
/* Generate cmdline_info for legacy flags and kernel-named
* features, and parse our arguments.
*/
- cmdline_features = calloc(ARRAY_SIZE(off_flag_def) + defs->n_features,
+ cmdline_features = calloc(2 * ARRAY_SIZE(off_flag_def) +
+ defs->n_features,
sizeof(cmdline_features[0]));
if (!cmdline_features) {
perror("Cannot parse arguments");
return 1;
}
- for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
+ for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
flag_to_cmdline_info(off_flag_def[i].short_name,
off_flag_def[i].value,
&off_flags_wanted, &off_flags_mask,
- &cmdline_features[i]);
+ &cmdline_features[2 * i]);
+ flag_to_cmdline_info(off_flag_def[i].long_name,
+ off_flag_def[i].value,
+ &off_flags_wanted, &off_flags_mask,
+ &cmdline_features[2 * i] + 1);
+ }
for (i = 0; i < defs->n_features; i++)
flag_to_cmdline_info(
defs->def[i].name, FEATURE_FIELD_FLAG(i),
&FEATURE_WORD(efeatures->features, i, requested),
&FEATURE_WORD(efeatures->features, i, valid),
- &cmdline_features[ARRAY_SIZE(off_flag_def) + i]);
+ &cmdline_features[2 * ARRAY_SIZE(off_flag_def) + i]);
parse_generic_cmdline(ctx, &any_changed, cmdline_features,
- ARRAY_SIZE(off_flag_def) + defs->n_features);
+ 2 * ARRAY_SIZE(off_flag_def) + defs->n_features);
free(cmdline_features);
if (!any_changed) {
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply related
* Re: [PATCH net-next v2 1/4] net: allow > 0 order atomic page alloc in skb_page_frag_refill
From: Debabrata Banerjee @ 2014-01-08 21:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: Michael Dalton, Michael S. Tsirkin, netdev@vger.kernel.org,
jbaron, virtualization, Eric Dumazet, Joshua Hunt,
David S. Miller
In-Reply-To: <1389210371.31367.8.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Jan 8, 2014 at 2:46 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Wed, 2014-01-08 at 21:18 +0200, Michael S. Tsirkin wrote:
>> On Wed, Jan 08, 2014 at 10:26:03AM -0800, Eric Dumazet wrote:
>> > On Wed, 2014-01-08 at 20:08 +0200, Michael S. Tsirkin wrote:
>> >
>> > > Eric said we also need a patch to add __GFP_NORETRY, right?
>> > > Probably before this one in series.
>> >
>> > Nope, this __GFP_NORETRY has nothing to do with this.
>> >
>> > I am not yet convinced we want it.
>> >
>> > This needs mm guys advice, as its a tradeoff for mm layer more than
>> > networking...
>>
>> Well maybe Cc linux-mm then?
>
> Well, I do not care of people mlocking the memory and complaining that
> compaction does not work.
>
> If these people care, they should contact mm guys, eventually.
>
> Really this is an issue that has nothing to do with this patch set.
>
Actually I have more data on this:
1. __GFP_NORETRY really does help and should go into stable tree.
2. You may want to consider GFP_NOKSWAPD, because even in the
GFP_ATOMIC case you are waking up kswapd to do reclaims on a
continuous basis even when you don't enter direct reclaim.
3. mlocking memory had very little to do with it, that was a
red-herring. I tested out the problem scenario with no mlocks. You
simply need memory pressure from page_cache, and mm ends up constantly
reclaiming and trying to keep another 1-2GB free on our systems (8GB
phys ~4GB left for kernel, ~3GB optimally used for page_cache).
4. I think perhaps using a kmem_cache allocation for this buffer is
the right way to make this work. I am experimenting with a patch to do
this.
-Debabrata
-Debabrata
^ permalink raw reply
* Re: [PATCH net-next V3 1/3] net: Add GRO support for UDP encapsulating protocols
From: Tom Herbert @ 2014-01-08 21:58 UTC (permalink / raw)
To: Or Gerlitz
Cc: Jerry Chu, Eric Dumazet, Herbert Xu, Linux Netdev List,
David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <1389213278-2200-2-git-send-email-ogerlitz@mellanox.com>
On Wed, Jan 8, 2014 at 12:34 PM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> Add GRO handlers for protocols that do UDP encapsulation, with the intent of
> being able to coalesce packets which encapsulate packets belonging to
> the same TCP session.
>
> For GRO purposes, the destination UDP port takes the role of the ether type
> field in the ethernet header or the next protocol in the IP header.
>
> The UDP GRO handler will only attempt to coalesce packets whose destination
> port is registered to have gro handler.
>
> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> include/linux/netdevice.h | 10 +++-
> include/net/protocol.h | 3 +
> net/core/dev.c | 1 +
> net/ipv4/udp_offload.c | 129 +++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 142 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index a2a70cc..360551a 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1652,7 +1652,9 @@ struct napi_gro_cb {
> unsigned long age;
>
> /* Used in ipv6_gro_receive() */
> - int proto;
> + u16 proto;
> +
> + u16 udp_mark;
>
> /* used to support CHECKSUM_COMPLETE for tunneling protocols */
> __wsum csum;
> @@ -1691,6 +1693,12 @@ struct packet_offload {
> struct list_head list;
> };
>
> +struct udp_offload {
> + __be16 port;
> + struct offload_callbacks callbacks;
> + struct list_head list;
> +};
> +
> /* often modified stats are per cpu, other are shared (netdev->stats) */
> struct pcpu_sw_netstats {
> u64 rx_packets;
> diff --git a/include/net/protocol.h b/include/net/protocol.h
> index fbf7676..fe9af94 100644
> --- a/include/net/protocol.h
> +++ b/include/net/protocol.h
> @@ -103,6 +103,9 @@ int inet_del_offload(const struct net_offload *prot, unsigned char num);
> void inet_register_protosw(struct inet_protosw *p);
> void inet_unregister_protosw(struct inet_protosw *p);
>
> +void udp_add_offload(struct udp_offload *prot);
> +void udp_del_offload(struct udp_offload *prot);
> +
> #if IS_ENABLED(CONFIG_IPV6)
> int inet6_add_protocol(const struct inet6_protocol *prot, unsigned char num);
> int inet6_del_protocol(const struct inet6_protocol *prot, unsigned char num);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index ce01847..11f7acf 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3858,6 +3858,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
> NAPI_GRO_CB(skb)->same_flow = 0;
> NAPI_GRO_CB(skb)->flush = 0;
> NAPI_GRO_CB(skb)->free = 0;
> + NAPI_GRO_CB(skb)->udp_mark = 0;
>
> pp = ptype->callbacks.gro_receive(&napi->gro_list, skb);
> break;
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 79c62bd..2846ade 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -13,6 +13,16 @@
> #include <linux/skbuff.h>
> #include <net/udp.h>
> #include <net/protocol.h>
> +/*
> +struct udp_offload {
> + __be16 port;
> + struct offload_callbacks callbacks;
> + struct list_head list;
> +};
> +*/
> +
> +static DEFINE_SPINLOCK(udp_offload_lock);
> +static struct list_head udp_offload_base __read_mostly;
>
> static int udp4_ufo_send_check(struct sk_buff *skb)
> {
> @@ -89,14 +99,133 @@ out:
> return segs;
> }
>
> +void udp_add_offload(struct udp_offload *uo)
> +{
> + struct list_head *head = &udp_offload_base;
> +
> + spin_lock(&udp_offload_lock);
> + list_add_rcu(&uo->list, head);
> + spin_unlock(&udp_offload_lock);
> +}
> +EXPORT_SYMBOL(udp_add_offload);
> +
> +void udp_del_offload(struct udp_offload *uo)
> +{
> + struct list_head *head = &udp_offload_base;
> + struct udp_offload *uo1;
> +
> + spin_lock(&udp_offload_lock);
> + list_for_each_entry(uo1, head, list) {
> + if (uo == uo1) {
> + list_del_rcu(&uo->list);
> + goto out;
> + }
> + }
> +
> + pr_warn("udp_remove_offload: %p not found port %d\n", uo, htons(uo->port));
> +out:
> + spin_unlock(&udp_offload_lock);
> +
> + synchronize_net();
> +}
> +EXPORT_SYMBOL(udp_del_offload);
> +
> +static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
> +{
> + struct list_head *ohead = &udp_offload_base;
> + struct udp_offload *poffload;
> + struct sk_buff *p, **pp = NULL;
> + struct udphdr *uh, *uh2;
> + unsigned int hlen, off;
> + int flush = 1;
> +
> + if (NAPI_GRO_CB(skb)->udp_mark ||
> + (!skb->encapsulation && skb->ip_summed != CHECKSUM_COMPLETE))
> + goto out;
> +
> + /* mark that this skb passed once through the udp gro layer */
> + NAPI_GRO_CB(skb)->udp_mark = 1;
> +
> + off = skb_gro_offset(skb);
> + hlen = off + sizeof(*uh);
> + uh = skb_gro_header_fast(skb, off);
> + if (skb_gro_header_hard(skb, hlen)) {
> + uh = skb_gro_header_slow(skb, hlen, off);
> + if (unlikely(!uh))
> + goto out;
> + }
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(poffload, ohead, list) {
> + if (poffload->port != uh->dest || !poffload->callbacks.gro_receive)
Is gro_receive == NULL ever valid? Maybe we can assert on registration
instead of checking on every packet.
Maybe make this poffload->port == uh->dest and goto "flush = 0".
Check below that list end was reached becomes unnecessary.
> + continue;
> + break;
> + }
> +
> + if (&poffload->list == ohead)
> + goto out_unlock;
> +
> + flush = 0;
> +
> + for (p = *head; p; p = p->next) {
> + if (!NAPI_GRO_CB(p)->same_flow)
> + continue;
> +
> + uh2 = (struct udphdr *)(p->data + off);
> + if ((*(u32 *)&uh->source != *(u32 *)&uh2->source)) {
> + NAPI_GRO_CB(p)->same_flow = 0;
> + continue;
> + }
> + goto found;
> + }
> +
> +found:
> + skb_gro_pull(skb, sizeof(struct udphdr)); /* pull encapsulating udp header */
> + pp = poffload->callbacks.gro_receive(head, skb);
> +
> +out_unlock:
> + rcu_read_unlock();
> +out:
> + NAPI_GRO_CB(skb)->flush |= flush;
> +
> + return pp;
> +}
> +
> +static int udp_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> + struct list_head *ohead = &udp_offload_base;
> + struct udp_offload *poffload;
> + __be16 newlen = htons(skb->len - nhoff);
> + struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
> + int err = -ENOSYS;
> +
> + uh->len = newlen;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(poffload, ohead, list) {
> + if (poffload->port != uh->dest || !poffload->callbacks.gro_complete)
> + continue;
> + break;
> + }
> +
> + if (&poffload->list != ohead)
> + err = poffload->callbacks.gro_complete(skb, nhoff + sizeof(struct udphdr));
> +
> + rcu_read_unlock();
> + return err;
> +}
> +
> static const struct net_offload udpv4_offload = {
> .callbacks = {
> .gso_send_check = udp4_ufo_send_check,
> .gso_segment = udp4_ufo_fragment,
> + .gro_receive = udp_gro_receive,
> + .gro_complete = udp_gro_complete,
> },
> };
>
> int __init udpv4_offload_init(void)
> {
> + INIT_LIST_HEAD(&udp_offload_base);
> return inet_add_offload(&udpv4_offload, IPPROTO_UDP);
> }
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* RE: [PATCH ethtool] ethtool: Accept long feature names reported by -k option as input to -K option
From: Allan, Bruce W @ 2014-01-08 22:00 UTC (permalink / raw)
To: Ben Hutchings, Or Gerlitz, Bill Fink, netdev@vger.kernel.org
In-Reply-To: <1389217931.1644.44.camel@bwh-desktop.uk.level5networks.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Ben Hutchings
> Sent: Wednesday, January 08, 2014 1:52 PM
> To: Or Gerlitz; Bill Fink; netdev@vger.kernel.org
> Subject: [PATCH ethtool] ethtool: Accept long feature names reported by -k
> option as input to -K option
>
> Before the generic features API was introduced, the ethtool -K option
> took short names for various features, e.g. 'gso' but the -k option
> reported their state using longer names,
> e.g. 'generic-segmentation-offload'.
>
> All newer features have a single kernel-provided name so input
> and output are consistent. But the old features still aren't, and
> although their short names are documented it's not good to have
> these exceptions.
>
> Change the argument parsing code for -K so that the long names
> reported by -k are also accepted.
>
> Reported-by: Or Gerlitz <or.gerlitz@gmail.com>
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> This turned out to be pretty easy to do as the argument processing is
> all table-driven already. I'll push this if it works for you.
>
> Ben.
>
> ethtool.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
Will there be an update to the man page, too?
^ permalink raw reply
* Re: [PATCH net-next v2 1/4] net: allow > 0 order atomic page alloc in skb_page_frag_refill
From: Eric Dumazet @ 2014-01-08 22:01 UTC (permalink / raw)
To: Debabrata Banerjee
Cc: Michael Dalton, Michael S. Tsirkin, netdev@vger.kernel.org,
jbaron, virtualization, Eric Dumazet, Joshua Hunt,
David S. Miller
In-Reply-To: <CAATkVEypFGfXuiBwFacOMjAWyYmLXHiihdpQfJp+CRFEZJagyg@mail.gmail.com>
On Wed, 2014-01-08 at 16:54 -0500, Debabrata Banerjee wrote:
> Actually I have more data on this:
>
Could you please stop polluting this thread ?
> 1. __GFP_NORETRY really does help and should go into stable tree.
>
Not at all. You are free to patch your kernel if you want.
It helps you workload, and breaks all others.
If compaction is never triggered, we'll never be able to get high order
pages, and performance goes back to what we had 2 years ago.
That discussion does not belong to this thread, again.
> 2. You may want to consider GFP_NOKSWAPD, because even in the
> GFP_ATOMIC case you are waking up kswapd to do reclaims on a
> continuous basis even when you don't enter direct reclaim.
>
> 3. mlocking memory had very little to do with it, that was a
> red-herring. I tested out the problem scenario with no mlocks. You
> simply need memory pressure from page_cache, and mm ends up constantly
> reclaiming and trying to keep another 1-2GB free on our systems (8GB
> phys ~4GB left for kernel, ~3GB optimally used for page_cache).
>
> 4. I think perhaps using a kmem_cache allocation for this buffer is
> the right way to make this work. I am experimenting with a patch to do
> this.
Seriously... I think you missed whole point of having frag allocation on
pages, not kmem_cache.
^ permalink raw reply
* Re: [PATCH net-next V3 3/3] net: Add GRO support for vxlan traffic
From: Eric Dumazet @ 2014-01-08 22:09 UTC (permalink / raw)
To: Or Gerlitz; +Cc: hkchu, edumazet, herbert, netdev, davem, yanb, shlomop
In-Reply-To: <1389213278-2200-4-git-send-email-ogerlitz@mellanox.com>
On Wed, 2014-01-08 at 22:34 +0200, Or Gerlitz wrote:
> +
> +static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
> +{
> + struct ethhdr *eh;
> + struct packet_offload *ptype;
> + __be16 type;
> + /* 22 = 8 bytes for the vlxan header + 14 bytes for the inner eth header */
> + int vxlan_len = 22;
I am pretty sure this can use existing macros or sizeof(...)
^ permalink raw reply
* Re: [PATCH net-next V3 3/3] net: Add GRO support for vxlan traffic
From: Eric Dumazet @ 2014-01-08 22:11 UTC (permalink / raw)
To: Or Gerlitz; +Cc: hkchu, edumazet, herbert, netdev, davem, yanb, shlomop
In-Reply-To: <1389213278-2200-4-git-send-email-ogerlitz@mellanox.com>
On Wed, 2014-01-08 at 22:34 +0200, Or Gerlitz wrote:
> +
> /* Notify netdevs that UDP port started listening */
> -static void vxlan_notify_add_rx_port(struct sock *sk)
> +static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
> {
> struct net_device *dev;
> + struct sock *sk = vs->sock->sk;
> struct net *net = sock_net(sk);
> sa_family_t sa_family = sk->sk_family;
> __be16 port = inet_sk(sk)->inet_sport;
> @@ -569,12 +671,16 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
> port);
> }
> rcu_read_unlock();
> +
> + if (sa_family == AF_INET)
> + call_rcu(&vs->rcu, vxlan_add_udp_offload);
Why waiting RCU grace period here ?
> }
>
> /* Notify netdevs that UDP port is no more listening */
> -static void vxlan_notify_del_rx_port(struct sock *sk)
> +static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
> {
> struct net_device *dev;
> + struct sock *sk = vs->sock->sk;
> struct net *net = sock_net(sk);
> sa_family_t sa_family = sk->sk_family;
> __be16 port = inet_sk(sk)->inet_sport;
> @@ -586,6 +692,9 @@ static void vxlan_notify_del_rx_port(struct sock *sk)
> port);
> }
> rcu_read_unlock();
> +
> + if (sa_family == AF_INET)
> + call_rcu(&vs->rcu, vxlan_del_udp_offload);
> }
This looks buggy.
You need to :
1) remove the offload structure from list
2) Then wait rcu grace period, and finally free the memory
^ permalink raw reply
* Re: [PATCH net-next 1/3] net: add skb_checksum_setup
From: Eric Dumazet @ 2014-01-08 22:43 UTC (permalink / raw)
To: Paul Durrant
Cc: netdev, xen-devel, David Miller, Eric Dumazet, Veaceslav Falico,
Alexander Duyck, Nicolas Dichtel
In-Reply-To: <1389189511-14568-2-git-send-email-paul.durrant@citrix.com>
On Wed, 2014-01-08 at 13:58 +0000, Paul Durrant wrote:
> This patch adds a function to set up the partial checksum offset for IP
> packets (and optionally re-calculate the pseudo-header checksum) into the
> core network code.
> The implementation was previously private and duplicated between xen-netback
> and xen-netfront, however it is not xen-specific and is potentially useful
> to any network driver.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: David Miller <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Veaceslav Falico <vfalico@redhat.com>
> Cc: Alexander Duyck <alexander.h.duyck@intel.com>
> Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> ---
> include/linux/netdevice.h | 1 +
> net/core/dev.c | 271 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 272 insertions(+)
Is there any reason to put this in net/core/dev.c instead of
net/core/skbuff.c ?
Also, no inline should be used in a .c file
( skb_maybe_pull_tail )
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] tcp: metrics: Add source-address to tcp-metrics
From: Christoph Paasch @ 2014-01-08 22:43 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389203751.26646.100.camel@edumazet-glaptop2.roam.corp.google.com>
Hello Eric,
On 08/01/14 - 09:55:51, Eric Dumazet wrote:
> On Wed, 2014-01-08 at 16:05 +0100, Christoph Paasch wrote:
> > We add the source-address to the tcp-metrics, so that different metrics
> > will be used per source/destination-pair. We use the destination-hash to
> > store the metric inside the hash-table. That way, deleting and dumping
> > via "ip tcp_metrics" is easy.
>
> Note that this has the following problem :
>
> Some applications use a set of source IP addresses to overcome the 64K
> port limitation.
Ok, did not know about that.
> tcp_metrics uses a hard-coded TCP_METRICS_RECLAIM_DEPTH value of 5,
> meaning that cache wont be able to store more than 5 source IP addresses
> (reaching one particular remote IP).
Maybe we could do something like the below (yet untested). That way we allow
up to 32 entries with the same destination but different source and still
only 5 with different destinations.
I guess 32 * 64K connections is enough. :)
We could also make TCP_METRICS_RECLAIM_DEPTH(_DST) a tunable.
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 699a42faab9c..0418ac318e7d 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -181,13 +181,18 @@ static void tcpm_check_stamp(struct tcp_metrics_block *tm, struct dst_entry *dst
}
#define TCP_METRICS_RECLAIM_DEPTH 5
+#define TCP_METRICS_RECLAIM_DEPTH_DST 32
#define TCP_METRICS_RECLAIM_PTR (struct tcp_metrics_block *) 0x1UL
-static struct tcp_metrics_block *tcp_get_encode(struct tcp_metrics_block *tm, int depth)
+static struct tcp_metrics_block *tcp_get_encode(struct tcp_metrics_block *tm,
+ int depth_general,
+ int depth_dst)
{
if (tm)
return tm;
- if (depth > TCP_METRICS_RECLAIM_DEPTH)
+ if (depth_general > TCP_METRICS_RECLAIM_DEPTH)
+ return TCP_METRICS_RECLAIM_PTR;
+ if (depth_dst > TCP_METRICS_RECLAIM_DEPTH_DST)
return TCP_METRICS_RECLAIM_PTR;
return NULL;
}
@@ -197,16 +202,19 @@ static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *s
struct net *net, unsigned int hash)
{
struct tcp_metrics_block *tm;
- int depth = 0;
+ int depth_dst = 0, depth_general = 0;
for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
tm = rcu_dereference(tm->tcpm_next)) {
if (addr_same(&tm->tcpm_saddr, saddr) &&
addr_same(&tm->tcpm_daddr, daddr))
break;
- depth++;
+ if (addr_same(&tm->tcpm_daddr, daddr))
+ depth_dst++;
+ else
+ depth_general++;
}
- return tcp_get_encode(tm, depth);
+ return tcp_get_encode(tm, depth_general, depth_dst);
}
static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
^ permalink raw reply related
* Re: Bug: alx: Atheros AR8131/AR8151/AR8152/AR8161 Ethernet driver
From: Sam @ 2014-01-08 22:53 UTC (permalink / raw)
To: netdev
In-Reply-To: <CACtZaF+aamW_0P2O1zSGLWHmichAZKb96pC4GCCVZZXJsh8Y7g@mail.gmail.com>
Kinley Dorji <kinleyd <at> gmail.com> writes:
>
> <at> Johannes Berg: > don't really care much for the
> > stats in the system I'm using this device on - and based on the driver
> > I'm not sure I'd use the chip for 'serious' work anyway :)
>
> OTOH, conky users love having the stats up, even if they aren't precise. :)
>
> There's one other point:
>
> Before the mainlining of the alx drivers, the default settings for
> Wake-on used to be d, whereas now it seems to be pg. This startled me
> a couple of times with my computer turning on unexpectedly, having
> been used to the previous defaults. Just FYI.
>
> Other than that, I would like thank you all very much for your quick
> responses and I look forward to having the stats code restored to the
> alx drivers.
>
> With best regards,
>
> Kinley
>
Hi guys,
I am experiencing this bug on a 3.11 kernel.
Can you tell me if the fix has been released upstream? in which kernel version?
Thanks
Regards
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] tcp: metrics: Add source-address to tcp-metrics
From: Eric Dumazet @ 2014-01-08 23:13 UTC (permalink / raw)
To: Christoph Paasch; +Cc: netdev, David Miller, Yuchung Cheng, Julian Anastasov
In-Reply-To: <20140108224344.GE4700@cpaasch-mac>
On Wed, 2014-01-08 at 23:43 +0100, Christoph Paasch wrote:
> Hello Eric,
>
> On 08/01/14 - 09:55:51, Eric Dumazet wrote:
> > On Wed, 2014-01-08 at 16:05 +0100, Christoph Paasch wrote:
> > > We add the source-address to the tcp-metrics, so that different metrics
> > > will be used per source/destination-pair. We use the destination-hash to
> > > store the metric inside the hash-table. That way, deleting and dumping
> > > via "ip tcp_metrics" is easy.
> >
> > Note that this has the following problem :
> >
> > Some applications use a set of source IP addresses to overcome the 64K
> > port limitation.
>
> Ok, did not know about that.
>
> > tcp_metrics uses a hard-coded TCP_METRICS_RECLAIM_DEPTH value of 5,
> > meaning that cache wont be able to store more than 5 source IP addresses
> > (reaching one particular remote IP).
>
> Maybe we could do something like the below (yet untested). That way we allow
> up to 32 entries with the same destination but different source and still
> only 5 with different destinations.
>
> I guess 32 * 64K connections is enough. :)
> We could also make TCP_METRICS_RECLAIM_DEPTH(_DST) a tunable.
Well, not sure if this is a problem anyway, and if we want extra
complexity for this rare use case, considering tcp metrics for
high number of flows sharing a common path is unlikely to be useful
(with exception of Fast Open, but again it must be rare)
^ permalink raw reply
* [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <20140108200019.GK9007@order.stressinduktion.org>
v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
v2 -> v3: reword commit messages, code comments and some refactoring.
v3 -> v4: refactor, rename variables, add enum
Thomas Haller (2):
ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
IP6 routes
ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 203 ++++++++++++++++++++++++++-----------------
2 files changed, 123 insertions(+), 81 deletions(-)
--
1.8.4.2
^ permalink raw reply
* [PATCH v4 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389227404-12586-1-git-send-email-thaller@redhat.com>
When adding/modifying an IPv6 address, the userspace application needs
a way to suppress adding a prefix route. This is for example relevant
together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf
generated addresses, but depending on on-link, no route for the
prefix should be added.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index cfed10b..dea10a8 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -49,6 +49,7 @@ enum {
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
#define IFA_F_MANAGETEMPADDR 0x100
+#define IFA_F_NOPREFIXROUTE 0x200
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31f75ea..7f0f011 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
valid_lft, prefered_lft);
if (!IS_ERR(ifp)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
+ expires, flags);
+ }
+
/*
* Note that section 3.1 of RFC 4429 indicates
* that the Optimistic flag should not be set for
@@ -3658,7 +3661,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
- IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+ IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+ IFA_F_NOPREFIXROUTE);
ifp->flags |= ifa_flags;
ifp->tstamp = jiffies;
ifp->valid_lft = valid_lft;
@@ -3668,8 +3672,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
+ expires, flags);
+ }
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
@@ -3723,7 +3729,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
/* We ignore other flags so far. */
- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR;
+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+ IFA_F_NOPREFIXROUTE;
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v4 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389227404-12586-1-git-send-email-thaller@redhat.com>
Refactor the deletion/update of prefix routes when removing an
address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the route. Instead, assume
that userspace is taking care of this route.
Also perform the same cleanup, when userspace changes an existing address
to add NOPREFIXROUTE (to an address that didn't have this flag). This is
done because when the address was added, a prefix route was created for it.
Since the user now wants to handle this route by himself, we cleanup this
route.
This cleanup of the route is not totally robust. There is no guarantee,
that the route we are about to delete was really the one added by the
kernel. This behavior does not change by the patch, and in practice it
should work just fine.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
net/ipv6/addrconf.c | 184 +++++++++++++++++++++++++++++++---------------------
1 file changed, 109 insertions(+), 75 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7f0f011..f4e2b7a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,95 @@ out:
goto out2;
}
+enum cleanup_prefix_rt_t {
+ CLEANUP_PREFIX_RT_NOP, /* no cleanup action for prefix route */
+ CLEANUP_PREFIX_RT_DEL, /* delete the prefix route */
+ CLEANUP_PREFIX_RT_EXPIRE, /* update the lifetime of the prefix route */
+};
+
+/*
+ * Check, whether the prefix for ifp would still need a prefix route
+ * after deleting ifp. The function returns one of the CLEANUP_PREFIX_RT_*
+ * constants.
+ *
+ * 1) we don't purge prefix if address was not permanent.
+ * prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there're no addresses, delete prefix.
+ * 4) if there're still other permanent address(es),
+ * corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ * don't purge the prefix, assume user space is managing it.
+ * 6) otherwise, update prefix lifetime to the
+ * longest valid lifetime among the corresponding
+ * addresses on the device.
+ * Note: subsequent RA will update lifetime.
+ **/
+static enum cleanup_prefix_rt_t
+check_cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long *expires)
+{
+ struct inet6_ifaddr *ifa;
+ struct inet6_dev *idev = ifp->idev;
+ unsigned long lifetime;
+ enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_DEL;
+
+ *expires = jiffies;
+
+ list_for_each_entry(ifa, &idev->addr_list, if_list) {
+ if (ifa == ifp)
+ continue;
+ if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+ ifp->prefix_len))
+ continue;
+ if (ifa->flags & (IFA_F_PERMANENT | IFA_F_NOPREFIXROUTE))
+ return CLEANUP_PREFIX_RT_NOP;
+
+ action = CLEANUP_PREFIX_RT_EXPIRE;
+
+ spin_lock(&ifa->lock);
+
+ lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+ /*
+ * Note: Because this address is
+ * not permanent, lifetime <
+ * LONG_MAX / HZ here.
+ */
+ if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+ *expires = ifa->tstamp + lifetime * HZ;
+ spin_unlock(&ifa->lock);
+ }
+
+ return action;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, bool del_rt)
+{
+ struct rt6_info *rt;
+
+ rt = addrconf_get_prefix_route(&ifp->addr,
+ ifp->prefix_len,
+ ifp->idev->dev,
+ 0, RTF_GATEWAY | RTF_DEFAULT);
+ if (rt) {
+ if (del_rt)
+ ip6_del_rt(rt);
+ else {
+ if (!(rt->rt6i_flags & RTF_EXPIRES))
+ rt6_set_expires(rt, expires);
+ ip6_rt_put(rt);
+ }
+ }
+}
+
+
/* This function wants to get referenced ifp and releases it before return */
static void ipv6_del_addr(struct inet6_ifaddr *ifp)
{
- struct inet6_ifaddr *ifa, *ifn;
- struct inet6_dev *idev = ifp->idev;
int state;
- int deleted = 0, onlink = 0;
- unsigned long expires = jiffies;
+ enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_NOP;
+ unsigned long expires;
spin_lock_bh(&ifp->state_lock);
state = ifp->state;
@@ -922,7 +1002,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
hlist_del_init_rcu(&ifp->addr_lst);
spin_unlock_bh(&addrconf_hash_lock);
- write_lock_bh(&idev->lock);
+ write_lock_bh(&ifp->idev->lock);
if (ifp->flags&IFA_F_TEMPORARY) {
list_del(&ifp->tmp_list);
@@ -933,45 +1013,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
__in6_ifa_put(ifp);
}
- list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
- if (ifa == ifp) {
- list_del_init(&ifp->if_list);
- __in6_ifa_put(ifp);
+ if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE))
+ action = check_cleanup_prefix_route(ifp, &expires);
- if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
- break;
- deleted = 1;
- continue;
- } else if (ifp->flags & IFA_F_PERMANENT) {
- if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
- ifp->prefix_len)) {
- if (ifa->flags & IFA_F_PERMANENT) {
- onlink = 1;
- if (deleted)
- break;
- } else {
- unsigned long lifetime;
-
- if (!onlink)
- onlink = -1;
-
- spin_lock(&ifa->lock);
-
- lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
- /*
- * Note: Because this address is
- * not permanent, lifetime <
- * LONG_MAX / HZ here.
- */
- if (time_before(expires,
- ifa->tstamp + lifetime * HZ))
- expires = ifa->tstamp + lifetime * HZ;
- spin_unlock(&ifa->lock);
- }
- }
- }
- }
- write_unlock_bh(&idev->lock);
+ list_del_init(&ifp->if_list);
+ __in6_ifa_put(ifp);
+
+ write_unlock_bh(&ifp->idev->lock);
addrconf_del_dad_timer(ifp);
@@ -979,38 +1027,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
- /*
- * Purge or update corresponding prefix
- *
- * 1) we don't purge prefix here if address was not permanent.
- * prefix is managed by its own lifetime.
- * 2) if there're no addresses, delete prefix.
- * 3) if there're still other permanent address(es),
- * corresponding prefix is still permanent.
- * 4) otherwise, update prefix lifetime to the
- * longest valid lifetime among the corresponding
- * addresses on the device.
- * Note: subsequent RA will update lifetime.
- *
- * --yoshfuji
- */
- if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
- struct rt6_info *rt;
-
- rt = addrconf_get_prefix_route(&ifp->addr,
- ifp->prefix_len,
- ifp->idev->dev,
- 0, RTF_GATEWAY | RTF_DEFAULT);
-
- if (rt) {
- if (onlink == 0) {
- ip6_del_rt(rt);
- rt = NULL;
- } else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
- rt6_set_expires(rt, expires);
- }
- }
- ip6_rt_put(rt);
+ if (action != CLEANUP_PREFIX_RT_NOP) {
+ cleanup_prefix_route(ifp, expires,
+ action == CLEANUP_PREFIX_RT_DEL);
}
/* clean up prefsrc entries */
@@ -3632,6 +3651,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
clock_t expires;
unsigned long timeout;
bool was_managetempaddr;
+ bool had_prefixroute;
if (!valid_lft || (prefered_lft > valid_lft))
return -EINVAL;
@@ -3660,6 +3680,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+ had_prefixroute = ifp->flags & IFA_F_PERMANENT &&
+ !(ifp->flags & IFA_F_NOPREFIXROUTE);
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
IFA_F_NOPREFIXROUTE);
@@ -3675,6 +3697,18 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
expires, flags);
+ } else if (had_prefixroute) {
+ enum cleanup_prefix_rt_t action;
+ unsigned long rt_expires;
+
+ write_lock_bh(&ifp->idev->lock);
+ action = check_cleanup_prefix_route(ifp, &rt_expires);
+ write_unlock_bh(&ifp->idev->lock);
+
+ if (action != CLEANUP_PREFIX_RT_NOP) {
+ cleanup_prefix_route(ifp, rt_expires,
+ action == CLEANUP_PREFIX_RT_DEL);
+ }
}
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH net-next 0/3] bonding: cleanup bond_3ad.c
From: Ding Tianhong @ 2014-01-09 1:36 UTC (permalink / raw)
To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389196008-28578-1-git-send-email-vfalico@redhat.com>
On 2014/1/8 23:46, Veaceslav Falico wrote:
> It's a huge mess there currently - and, thus, really hard to read and
> debug.
>
> This is the first series, and doesn't change the logic at all, only makes
> it a bit more readable.
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>
> ---
> drivers/net/bonding/bond_3ad.c | 731 +++++++++++++++++++++--------------------
> 1 file changed, 366 insertions(+), 365 deletions(-)
>
Great work!
Tested-by: Ding Tianhong <dingtianhong@huawei.com>
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael S. Tsirkin @ 2014-01-09 1:42 UTC (permalink / raw)
To: Michael Dalton; +Cc: netdev, virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <1389072355-20666-3-git-send-email-mwdalton@google.com>
On Mon, Jan 06, 2014 at 09:25:54PM -0800, Michael Dalton wrote:
> Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page frag
> allocators") changed the mergeable receive buffer size from PAGE_SIZE to
> MTU-size, introducing a single-stream regression for benchmarks with large
> average packet size. There is no single optimal buffer size for all
> workloads. For workloads with packet size <= MTU bytes, MTU + virtio-net
> header-sized buffers are preferred as larger buffers reduce the TCP window
> due to SKB truesize. However, single-stream workloads with large average
> packet sizes have higher throughput if larger (e.g., PAGE_SIZE) buffers
> are used.
>
> This commit auto-tunes the mergeable receiver buffer packet size by
> choosing the packet buffer size based on an EWMA of the recent packet
> sizes for the receive queue. Packet buffer sizes range from MTU_SIZE +
> virtio-net header len to PAGE_SIZE. This improves throughput for
> large packet workloads, as any workload with average packet size >=
> PAGE_SIZE will use PAGE_SIZE buffers.
>
> These optimizations interact positively with recent commit
> ba275241030c ("virtio-net: coalesce rx frags when possible during rx"),
> which coalesces adjacent RX SKB fragments in virtio_net. The coalescing
> optimizations benefit buffers of any size.
>
> Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
> between two QEMU VMs on a single physical machine. Each VM has two VCPUs
> with all offloads & vhost enabled. All VMs and vhost threads run in a
> single 4 CPU cgroup cpuset, using cgroups to ensure that other processes
> in the system will not be scheduled on the benchmark CPUs. Trunk includes
> SKB rx frag coalescing.
>
> net-next w/ virtio_net before 2613af0ed18a (PAGE_SIZE bufs): 14642.85Gb/s
> net-next (MTU-size bufs): 13170.01Gb/s
> net-next + auto-tune: 14555.94Gb/s
>
> Jason Wang also reported a throughput increase on mlx4 from 22Gb/s
> using MTU-sized buffers to about 26Gb/s using auto-tuning.
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
Sorry that I didn't notice early, but there seems to be a bug here.
See below.
Also, I think we can simplify code, see suggestion below.
> ---
> v2: Add per-receive queue metadata ring to track precise truesize for
> mergeable receive buffers. Remove all truesize approximation. Never
> try to fill a full RX ring (required for metadata ring in v2).
>
> drivers/net/virtio_net.c | 145 ++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 107 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 526dfd8..f6e1ee0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -26,6 +26,7 @@
> #include <linux/if_vlan.h>
> #include <linux/slab.h>
> #include <linux/cpu.h>
> +#include <linux/average.h>
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -36,11 +37,15 @@ module_param(gso, bool, 0444);
>
> /* FIXME: MTU in config. */
> #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> -#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
> - sizeof(struct virtio_net_hdr_mrg_rxbuf), \
> - L1_CACHE_BYTES))
> #define GOOD_COPY_LEN 128
>
> +/* Weight used for the RX packet size EWMA. The average packet size is used to
> + * determine the packet buffer size when refilling RX rings. As the entire RX
> + * ring may be refilled at once, the weight is chosen so that the EWMA will be
> + * insensitive to short-term, transient changes in packet size.
> + */
> +#define RECEIVE_AVG_WEIGHT 64
> +
> #define VIRTNET_DRIVER_VERSION "1.0.0"
>
> struct virtnet_stats {
> @@ -65,11 +70,30 @@ struct send_queue {
> char name[40];
> };
>
> +/* Per-packet buffer context for mergeable receive buffers. */
> +struct mergeable_receive_buf_ctx {
> + /* Packet buffer base address. */
> + void *buf;
> +
> + /* Original size of the packet buffer for use in SKB truesize. Does not
> + * include any padding space used to avoid internal fragmentation.
> + */
> + unsigned int truesize;
Don't need full int really, it's up to 4K/cache line size,
1 byte would be enough, maximum 2 ...
So if all we want is extra 1-2 bytes per buffer, we don't really
need this extra level of indirection I think.
We can just allocate them before the header together with an skb.
> +};
> +
> /* Internal representation of a receive virtqueue */
> struct receive_queue {
> /* Virtqueue associated with this receive_queue */
> struct virtqueue *vq;
>
> + /* Circular buffer of mergeable rxbuf contexts. */
> + struct mergeable_receive_buf_ctx *mrg_buf_ctx;
> +
> + /* Number of elements & head index of mrg_buf_ctx. Size must be
> + * equal to the associated virtqueue's vring size.
> + */
> + unsigned int mrg_buf_ctx_size, mrg_buf_ctx_head;
> +
> struct napi_struct napi;
>
> /* Number of input buffers, and max we've ever had. */
> @@ -78,6 +102,9 @@ struct receive_queue {
> /* Chain pages by the private ptr. */
> struct page *pages;
>
> + /* Average packet length for mergeable receive buffers. */
> + struct ewma mrg_avg_pkt_len;
> +
> /* Page frag for packet buffer allocation. */
> struct page_frag alloc_frag;
>
> @@ -327,32 +354,32 @@ err:
>
> static struct sk_buff *receive_mergeable(struct net_device *dev,
> struct receive_queue *rq,
> - void *buf,
> + struct mergeable_receive_buf_ctx *ctx,
> unsigned int len)
> {
> - struct skb_vnet_hdr *hdr = buf;
> + struct skb_vnet_hdr *hdr = ctx->buf;
> int num_buf = hdr->mhdr.num_buffers;
> - struct page *page = virt_to_head_page(buf);
> - int offset = buf - page_address(page);
> - unsigned int truesize = max_t(unsigned int, len, MERGE_BUFFER_LEN);
> + struct page *page = virt_to_head_page(ctx->buf);
> + int offset = ctx->buf - page_address(page);
> + unsigned int truesize = max(len, ctx->truesize);
> +
> struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, truesize);
> struct sk_buff *curr_skb = head_skb;
>
> if (unlikely(!curr_skb))
> goto err_skb;
> -
> while (--num_buf) {
> int num_skb_frags;
>
> - buf = virtqueue_get_buf(rq->vq, &len);
> - if (unlikely(!buf)) {
> + ctx = virtqueue_get_buf(rq->vq, &len);
> + if (unlikely(!ctx)) {
> pr_debug("%s: rx error: %d buffers out of %d missing\n",
> dev->name, num_buf, hdr->mhdr.num_buffers);
> dev->stats.rx_length_errors++;
> goto err_buf;
> }
>
> - page = virt_to_head_page(buf);
> + page = virt_to_head_page(ctx->buf);
> --rq->num;
>
> num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> @@ -369,13 +396,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> head_skb->truesize += nskb->truesize;
> num_skb_frags = 0;
> }
> - truesize = max_t(unsigned int, len, MERGE_BUFFER_LEN);
> + truesize = max(len, ctx->truesize);
> if (curr_skb != head_skb) {
> head_skb->data_len += len;
> head_skb->len += len;
> head_skb->truesize += truesize;
> }
> - offset = buf - page_address(page);
> + offset = ctx->buf - page_address(page);
> if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
> put_page(page);
> skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
> @@ -386,19 +413,20 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> }
> }
>
> + ewma_add(&rq->mrg_avg_pkt_len, head_skb->len);
> return head_skb;
>
> err_skb:
> put_page(page);
> while (--num_buf) {
> - buf = virtqueue_get_buf(rq->vq, &len);
> - if (unlikely(!buf)) {
> + ctx = virtqueue_get_buf(rq->vq, &len);
> + if (unlikely(!ctx)) {
> pr_debug("%s: rx error: %d buffers missing\n",
> dev->name, num_buf);
> dev->stats.rx_length_errors++;
> break;
> }
> - page = virt_to_head_page(buf);
> + page = virt_to_head_page(ctx->buf);
> put_page(page);
> --rq->num;
> }
> @@ -419,12 +447,14 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> dev->stats.rx_length_errors++;
> - if (vi->mergeable_rx_bufs)
> - put_page(virt_to_head_page(buf));
> - else if (vi->big_packets)
> + if (vi->mergeable_rx_bufs) {
> + struct mergeable_receive_buf_ctx *ctx = buf;
> + put_page(virt_to_head_page(ctx->buf));
> + } else if (vi->big_packets) {
> give_pages(rq, buf);
> - else
> + } else {
> dev_kfree_skb(buf);
> + }
> return;
> }
>
> @@ -572,29 +602,43 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
>
> static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> {
> + const unsigned int ring_size = rq->mrg_buf_ctx_size;
> + const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> struct page_frag *alloc_frag = &rq->alloc_frag;
> - char *buf;
> + struct mergeable_receive_buf_ctx *ctx;
> int err;
> unsigned int len, hole;
>
> - if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
> + len = hdr_len + clamp_t(unsigned int, ewma_read(&rq->mrg_avg_pkt_len),
> + GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
> + len = ALIGN(len, L1_CACHE_BYTES);
> + if (unlikely(!skb_page_frag_refill(len, alloc_frag, gfp)))
> return -ENOMEM;
> - buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> +
> + ctx = &rq->mrg_buf_ctx[rq->mrg_buf_ctx_head];
> + ctx->buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> + ctx->truesize = len;
> get_page(alloc_frag->page);
> - len = MERGE_BUFFER_LEN;
> alloc_frag->offset += len;
> hole = alloc_frag->size - alloc_frag->offset;
> - if (hole < MERGE_BUFFER_LEN) {
> + if (hole < len) {
> + /* To avoid internal fragmentation, if there is very likely not
> + * enough space for another buffer, add the remaining space to
> + * the current buffer. This extra space is not included in
> + * ctx->truesize.
> + */
> len += hole;
> alloc_frag->offset += hole;
> }
>
> - sg_init_one(rq->sg, buf, len);
> - err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> - if (err < 0)
> - put_page(virt_to_head_page(buf));
> -
> - return err;
> + sg_init_one(rq->sg, ctx->buf, len);
> + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, ctx, gfp);
> + if (err < 0) {
> + put_page(virt_to_head_page(ctx->buf));
> + return err;
> + }
> + rq->mrg_buf_ctx_head = (rq->mrg_buf_ctx_head + 1) & (ring_size - 1);
Wait a second. this assumes that buffers are consumes in order?
This happens to be the case but is not guaranteed by the spec
at all.
> + return 0;
> }
>
> /*
> @@ -610,6 +654,9 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
> int err;
> bool oom;
>
> + /* Do not attempt to add a buffer if the RX ring is full. */
> + if (unlikely(!rq->vq->num_free))
> + return true;
> gfp |= __GFP_COLD;
> do {
> if (vi->mergeable_rx_bufs)
> @@ -1354,8 +1401,10 @@ static void virtnet_free_queues(struct virtnet_info *vi)
> {
> int i;
>
> - for (i = 0; i < vi->max_queue_pairs; i++)
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> netif_napi_del(&vi->rq[i].napi);
> + kfree(vi->rq[i].mrg_buf_ctx);
> + }
>
> kfree(vi->rq);
> kfree(vi->sq);
> @@ -1394,12 +1443,14 @@ static void free_unused_bufs(struct virtnet_info *vi)
> struct virtqueue *vq = vi->rq[i].vq;
>
> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> - if (vi->mergeable_rx_bufs)
> - put_page(virt_to_head_page(buf));
> - else if (vi->big_packets)
> + if (vi->mergeable_rx_bufs) {
> + struct mergeable_receive_buf_ctx *ctx = buf;
> + put_page(virt_to_head_page(ctx->buf));
> + } else if (vi->big_packets) {
> give_pages(&vi->rq[i], buf);
> - else
> + } else {
> dev_kfree_skb(buf);
> + }
> --vi->rq[i].num;
> }
> BUG_ON(vi->rq[i].num != 0);
> @@ -1509,6 +1560,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> napi_weight);
>
> sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
> + ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
> sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
> }
>
> @@ -1522,7 +1574,8 @@ err_sq:
>
> static int init_vqs(struct virtnet_info *vi)
> {
> - int ret;
> + struct virtio_device *vdev = vi->vdev;
> + int i, ret;
>
> /* Allocate send & receive queues */
> ret = virtnet_alloc_queues(vi);
> @@ -1533,12 +1586,28 @@ static int init_vqs(struct virtnet_info *vi)
> if (ret)
> goto err_free;
>
> + if (vi->mergeable_rx_bufs) {
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + struct receive_queue *rq = &vi->rq[i];
> + rq->mrg_buf_ctx_size = virtqueue_get_vring_size(rq->vq);
> + rq->mrg_buf_ctx = kmalloc(sizeof(*rq->mrg_buf_ctx) *
> + rq->mrg_buf_ctx_size,
> + GFP_KERNEL);
> + if (!rq->mrg_buf_ctx) {
> + ret = -ENOMEM;
> + goto err_del_vqs;
> + }
> + }
> + }
> +
> get_online_cpus();
> virtnet_set_affinity(vi);
> put_online_cpus();
>
> return 0;
>
> +err_del_vqs:
> + vdev->config->del_vqs(vdev);
> err_free:
> virtnet_free_queues(vi);
> err:
> --
> 1.8.5.1
^ permalink raw reply
* linux-next: manual merge of the ipsec-next tree with the net-next tree
From: Stephen Rothwell @ 2014-01-09 2:41 UTC (permalink / raw)
To: Steffen Klassert, David Miller, netdev
Cc: linux-next, linux-kernel, Daniel Borkmann, Weilong Chen
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
Hi Steffen,
Today's linux-next merge of the ipsec-next tree got a conflict in
net/xfrm/xfrm_policy.c between commits be7928d20bab ("net: xfrm:
xfrm_policy: fix inline not at beginning of declaration") and
da7c224b1baa ("net: xfrm: xfrm_policy: silence compiler warning") from
the net-next tree and commit 2f3ea9a95c58 ("xfrm: checkpatch erros with
inline keyword position") from the ipsec-next tree.
I fixed it up (I used the version from the net-next tree) and can carry
the fix as necessary (no action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Jerry Chu @ 2014-01-09 3:12 UTC (permalink / raw)
To: Or Gerlitz
Cc: Or Gerlitz, Eric Dumazet, Eric Dumazet, Herbert Xu,
netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <52CD0626.9030001@mellanox.com>
On Wed, Jan 8, 2014 at 12:02 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> On 08/01/2014 00:11, Jerry Chu wrote:
>>
>> On Tue, Jan 7, 2014 at 12:37 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>>
>>> On Tue, Jan 7, 2014 at 10:32 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>> wrote:
>>>>
>>>> On Tue, 2014-01-07 at 22:19 +0200, Or Gerlitz wrote:
>>>>>
>>>>> On Tue, Jan 7, 2014 at 6:33 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>>>>>>>
>>>>>>> +
>>>>>>> +#define MAX_UDP_PORT (1 << 16)
>>>>>>> +extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
>>>>>>
>>>>>> Thats 512 KB of memory.
>>>>>> This will greatly impact forwarding performance of UDP packets with
>>>>>> random ports, and will increase kernel memory size for embedded
>>>>>> devices.
>>>>>
>>>>> Re forwarding, are you referring to the case where the forwarded
>>>>> packets are encapsulated? packets which are not encapusalted will be
>>>>> flushed in the gro receive handler (this went out by mistake in V2 but
>>>>> exists in V1) if skb->encapsulation isn't set.
>>>>>
>>>> How do you know encapsulation must be tried for a given incoming
>>>> packet ? NIC do not magically sets skb->encapsulation I think...
>>>
>>> So here's the thing, per my understanding we want to GRO only received
>>> **encapsulated** packets whose checksum status is != CHECKSUM_NONE
>>
>> What's wrong with GRO'ing pkts whose csum == CHECKSUM_NONE?
>
>
> I am not sure, intuitively it sounds a bit wrong to me, empirically, it
> doesn't work for udp encapsulated / vxlan
> traffic, I got drops from the tcp stack in tcp_rcv_established() -- if
> GRO-ed packets carry CHECKSUM_NONE
> we arrive to the csum_error label, which means that
> tcp_checksum_complete_user() failed for them
This is odd because if pkts have been aggregated successfully,
tcp4_gro_receive() should've skb_checksum() and turned CHECKSUM_NONE
into CHECKSUM_UNNECESSARY. (I think i've already tested this
case with my GRE-GRO patch on a NIC that sends up pkts w/ CHECKSUM_NONE.
But granted there are a lot of csum related bugs in the current code. I just
spent half a day scratching my head on a very low thruput number with
my GRE patch over a GRE tunnel w/ csum flag on. I just tracked it down
to be buggy TSO/GRE code that will produce bad csum on the tx side.
>
>
>
>> Also "udp_offload" is a little misleading - you are not trying to GRO UDP
>> pkts where UDP is the real transport. You are only trying to GRO UDP
>> encapped TCP pkts.
>
>
> Indeed -- however, I just plugged into what was there for GSO, e.g stack
> will not do GSO for plain UDP
> packets, only for those who encapsulate something the code that does this is
> udp_offloads.c -- any suggestion
> how to phrase/frame the change you envision?
There is already udpv4_offload for real udp gso (ufo) offload. How about
"udp_encap_offload" for your stuff?
>
>
>>
>
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 3:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <20140109014255.GA19321@redhat.com>
Hi Michael,
On Wed, Jan 8, 2014 at 5:42 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Sorry that I didn't notice early, but there seems to be a bug here.
> See below.
Yes, that is definitely a bug. Virtio spec permits OOO completions,
but current code assumes in-order completion. Thanks for catching this.
> Don't need full int really, it's up to 4K/cache line size,
> 1 byte would be enough, maximum 2 ...
> So if all we want is extra 1-2 bytes per buffer, we don't really
> need this extra level of indirection I think.
> We can just allocate them before the header together with an skb.
I'm not sure if I'm parsing the above correctly, but do you mean using a
few bytes at the beginning of the packet buffer to store truesize? I
think that will break Jason's virtio-net RX frag coalescing
code. To coalesce consecutive RX packet buffers, our packet buffers must
be physically adjacent, and any extra bytes before the start of the
buffer would break that.
We could allocate an SKB per packet buffer, but if we have multi-buffer
packets often(e.g., netperf benefiting from GSO/GRO), we would be
allocating 1 SKB per packet buffer instead of 1 SKB per MAX_SKB_FRAGS
buffers. How do you feel about any of the below alternatives:
(1) Modify the existing mrg_buf_ctx to chain together free entries
We can use the 'buf' pointer in mergeable_receive_buf_ctx to chain
together free entries so that we can support OOO completions. This would
be similar to how virtio-queue manages free sg entries.
(2) Combine the buffer pointer and truesize into a single void* value
Your point about there only being a byte needed to encode truesize is
spot on, and I think we could leverage this to eliminate the out-of-band
metadata ring entirely. If we were willing to change the packet buffer
alignment from L1_CACHE_BYTES to 256 (or min (256, L1_CACHE_SIZE)), we
could encode the truesize in the least significant 8 bits of the buffer
address (encoded as truesize >> 8 as we know all sizes are a multiple
of 256). This would allow packet buffers up to 64KB in length.
Is there another approach you would prefer to any of these? If the
cleanliness issues and larger alignment aren't too bad, I think (2)
sounds promising and allow us to eliminate the metadata ring
entirely while still permitting RX frag coalescing.
Best,
Mike
^ 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