* [PATCH RFC v2 net-next 0/2] ip_tunnel: Create percpu gro_cell
@ 2015-01-13 23:42 Martin KaFai Lau
2015-01-13 23:42 ` [PATCH RFC v2 net-next 1/2] " Martin KaFai Lau
2015-01-13 23:42 ` [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells Martin KaFai Lau
0 siblings, 2 replies; 7+ messages in thread
From: Martin KaFai Lau @ 2015-01-13 23:42 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, kernel-team
In the ipip tunnel, the skb->queue_mapping is lost in ipip_rcv().
All skb will be queued to the same cell->napi_skbs. The
gro_cell_poll is pinned to one core under load. In production traffic,
we also see severe rx_dropped in the tunl iface and it is probably due to
this limit: skb_queue_len(&cell->napi_skbs) > netdev_max_backlog
This patch is trying to alloc_percpu(struct gro_cell) and schedule
gro_cell_poll to process the skb in the same core.
Changes from v1:
Eric Dumazet pointed out that ____cacheline_aligned_in_smp is no longer needed.
Setup:
VIP_PREFIX=9.9.9.9/32
REMOTE_REAL_IP=10.228.95.75
if [ "$1" = "encap" ]
then
sudo ip tunnel add mode ipip remote ${REMOTE_REAL_IP}
sudo ip link set dev ipip0 up
sudo ip route add dev ipip0 ${VIP_PREFIX}
else
# Decapsulating host
sudo ip tunnel add mode ipip
sudo ip link set dev tunl0 up
sudo ip addr add dev lo ${VIP_PREFIX}
sudo sysctl -a | grep '\.rp_filter' | awk '{print $1;}' | \
xargs -n1 -I{} sudo sysctl {}=0
fi
Before:
[root@DECAP ~]# netserver -p 8888
[root@ENCAP ~]# super_netperf 200 -t TCP_RR -H 9.9.9.9 -p 8888 \
-l 30 -- -d 0x6 -m 8k,64k -s 1M -S 1M
332215
[root@DECAP ~]# perf probe -a gro_cell_poll
[root@DECAP ~]# perf stat -I 1000 -a -A -e probe:gro_cell_poll
117.258518273 CPU0 0 probe:gro_cell_poll
117.258518273 CPU1 0 probe:gro_cell_poll
117.258518273 CPU2 0 probe:gro_cell_poll
117.258518273 CPU3 0 probe:gro_cell_poll
117.258518273 CPU4 0 probe:gro_cell_poll
117.258518273 CPU5 0 probe:gro_cell_poll
117.258518273 CPU6 0 probe:gro_cell_poll
117.258518273 CPU7 0 probe:gro_cell_poll
117.258518273 CPU8 0 probe:gro_cell_poll
117.258518273 CPU9 0 probe:gro_cell_poll
117.258518273 CPU10 0 probe:gro_cell_poll
117.258518273 CPU11 0 probe:gro_cell_poll
117.258518273 CPU12 0 probe:gro_cell_poll
117.258518273 CPU13 0 probe:gro_cell_poll
117.258518273 CPU14 0 probe:gro_cell_poll
117.258518273 CPU15 4,882 probe:gro_cell_poll
117.258518273 CPU16 0 probe:gro_cell_poll
117.258518273 CPU17 0 probe:gro_cell_poll
117.258518273 CPU18 0 probe:gro_cell_poll
117.258518273 CPU19 0 probe:gro_cell_poll
117.258518273 CPU20 0 probe:gro_cell_poll
117.258518273 CPU21 0 probe:gro_cell_poll
117.258518273 CPU22 0 probe:gro_cell_poll
117.258518273 CPU23 0 probe:gro_cell_poll
117.258518273 CPU24 0 probe:gro_cell_poll
117.258518273 CPU25 0 probe:gro_cell_poll
117.258518273 CPU26 0 probe:gro_cell_poll
117.258518273 CPU27 0 probe:gro_cell_poll
117.258518273 CPU28 0 probe:gro_cell_poll
117.258518273 CPU29 0 probe:gro_cell_poll
117.258518273 CPU30 0 probe:gro_cell_poll
117.258518273 CPU31 0 probe:gro_cell_poll
117.258518273 CPU32 0 probe:gro_cell_poll
117.258518273 CPU33 0 probe:gro_cell_poll
117.258518273 CPU34 0 probe:gro_cell_poll
117.258518273 CPU35 0 probe:gro_cell_poll
117.258518273 CPU36 0 probe:gro_cell_poll
117.258518273 CPU37 0 probe:gro_cell_poll
117.258518273 CPU38 0 probe:gro_cell_poll
117.258518273 CPU39 0 probe:gro_cell_poll
After:
[root@DECAP ~]# netserver -p 8888
[root@ENCAP ~]# super_netperf 200 -t TCP_RR -H 9.9.9.9 -p 8888 \
-l 30 -- -d 0x6 -m 8k,64k -s 1M -S 1M
877530
[root@DECAP ~]# perf probe -a gro_cell_poll
[root@DECAP ~]# perf stat -I 1000 -a -A -e probe:gro_cell_poll
40.085714389 CPU0 13,607 probe:gro_cell_poll
40.085714389 CPU1 13,188 probe:gro_cell_poll
40.085714389 CPU2 12,913 probe:gro_cell_poll
40.085714389 CPU3 12,790 probe:gro_cell_poll
40.085714389 CPU4 13,395 probe:gro_cell_poll
40.085714389 CPU5 13,121 probe:gro_cell_poll
40.085714389 CPU6 11,083 probe:gro_cell_poll
40.085714389 CPU7 12,945 probe:gro_cell_poll
40.085714389 CPU8 13,704 probe:gro_cell_poll
40.085714389 CPU9 13,514 probe:gro_cell_poll
40.085714389 CPU10 0 probe:gro_cell_poll
40.085714389 CPU11 0 probe:gro_cell_poll
40.085714389 CPU12 0 probe:gro_cell_poll
40.085714389 CPU13 0 probe:gro_cell_poll
40.085714389 CPU14 0 probe:gro_cell_poll
40.085714389 CPU15 0 probe:gro_cell_poll
40.085714389 CPU16 0 probe:gro_cell_poll
40.085714389 CPU17 0 probe:gro_cell_poll
40.085714389 CPU18 0 probe:gro_cell_poll
40.085714389 CPU19 0 probe:gro_cell_poll
40.085714389 CPU20 10,402 probe:gro_cell_poll
40.085714389 CPU21 12,312 probe:gro_cell_poll
40.085714389 CPU22 11,913 probe:gro_cell_poll
40.085714389 CPU23 12,964 probe:gro_cell_poll
40.085714389 CPU24 13,727 probe:gro_cell_poll
40.085714389 CPU25 12,943 probe:gro_cell_poll
40.085714389 CPU26 13,558 probe:gro_cell_poll
40.085714389 CPU27 12,676 probe:gro_cell_poll
40.085714389 CPU28 13,754 probe:gro_cell_poll
40.085714389 CPU29 13,379 probe:gro_cell_poll
40.085714389 CPU30 0 probe:gro_cell_poll
40.085714389 CPU31 0 probe:gro_cell_poll
40.085714389 CPU32 0 probe:gro_cell_poll
40.085714389 CPU33 0 probe:gro_cell_poll
40.085714389 CPU34 0 probe:gro_cell_poll
40.085714389 CPU35 0 probe:gro_cell_poll
40.085714389 CPU36 0 probe:gro_cell_poll
40.085714389 CPU37 0 probe:gro_cell_poll
40.085714389 CPU38 0 probe:gro_cell_poll
40.085714389 CPU39 0 probe:gro_cell_poll
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH RFC v2 net-next 1/2] ip_tunnel: Create percpu gro_cell
2015-01-13 23:42 [PATCH RFC v2 net-next 0/2] ip_tunnel: Create percpu gro_cell Martin KaFai Lau
@ 2015-01-13 23:42 ` Martin KaFai Lau
2015-01-15 0:14 ` Eric Dumazet
2015-01-13 23:42 ` [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells Martin KaFai Lau
1 sibling, 1 reply; 7+ messages in thread
From: Martin KaFai Lau @ 2015-01-13 23:42 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, kernel-team
In the ipip tunnel, the skb->queue_mapping is lost in ipip_rcv().
All skb will be queued to the same cell->napi_skbs. The
gro_cell_poll is pinned to one core under load. In production traffic,
we also see severe rx_dropped in the tunl iface and it is probably due to
this limit: skb_queue_len(&cell->napi_skbs) > netdev_max_backlog.
This patch is trying to alloc_percpu(struct gro_cell) and schedule
gro_cell_poll to process the skb in the same core.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
include/net/gro_cells.h | 29 ++++++++++++-----------------
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/include/net/gro_cells.h b/include/net/gro_cells.h
index 734d9b5..0f712c0 100644
--- a/include/net/gro_cells.h
+++ b/include/net/gro_cells.h
@@ -8,25 +8,23 @@
struct gro_cell {
struct sk_buff_head napi_skbs;
struct napi_struct napi;
-} ____cacheline_aligned_in_smp;
+};
struct gro_cells {
- unsigned int gro_cells_mask;
- struct gro_cell *cells;
+ struct gro_cell __percpu *cells;
};
static inline void gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
{
- struct gro_cell *cell = gcells->cells;
+ struct gro_cell *cell;
struct net_device *dev = skb->dev;
- if (!cell || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
+ if (!gcells->cells || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
netif_rx(skb);
return;
}
- if (skb_rx_queue_recorded(skb))
- cell += skb_get_rx_queue(skb) & gcells->gro_cells_mask;
+ cell = this_cpu_ptr(gcells->cells);
if (skb_queue_len(&cell->napi_skbs) > netdev_max_backlog) {
atomic_long_inc(&dev->rx_dropped);
@@ -72,15 +70,12 @@ static inline int gro_cells_init(struct gro_cells *gcells, struct net_device *de
{
int i;
- gcells->gro_cells_mask = roundup_pow_of_two(netif_get_num_default_rss_queues()) - 1;
- gcells->cells = kcalloc(gcells->gro_cells_mask + 1,
- sizeof(struct gro_cell),
- GFP_KERNEL);
+ gcells->cells = alloc_percpu(struct gro_cell);
if (!gcells->cells)
return -ENOMEM;
- for (i = 0; i <= gcells->gro_cells_mask; i++) {
- struct gro_cell *cell = gcells->cells + i;
+ for_each_possible_cpu(i) {
+ struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
skb_queue_head_init(&cell->napi_skbs);
netif_napi_add(dev, &cell->napi, gro_cell_poll, 64);
@@ -91,16 +86,16 @@ static inline int gro_cells_init(struct gro_cells *gcells, struct net_device *de
static inline void gro_cells_destroy(struct gro_cells *gcells)
{
- struct gro_cell *cell = gcells->cells;
int i;
- if (!cell)
+ if (!gcells->cells)
return;
- for (i = 0; i <= gcells->gro_cells_mask; i++,cell++) {
+ for_each_possible_cpu(i) {
+ struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
netif_napi_del(&cell->napi);
skb_queue_purge(&cell->napi_skbs);
}
- kfree(gcells->cells);
+ free_percpu(gcells->cells);
gcells->cells = NULL;
}
--
1.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells
2015-01-13 23:42 [PATCH RFC v2 net-next 0/2] ip_tunnel: Create percpu gro_cell Martin KaFai Lau
2015-01-13 23:42 ` [PATCH RFC v2 net-next 1/2] " Martin KaFai Lau
@ 2015-01-13 23:42 ` Martin KaFai Lau
2015-01-14 22:53 ` Eric Dumazet
1 sibling, 1 reply; 7+ messages in thread
From: Martin KaFai Lau @ 2015-01-13 23:42 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, kernel-team
After adding percpu gro_cells, struct gro_cells can be removed.
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
include/net/gro_cells.h | 34 ++++++++++++++++------------------
include/net/ip_tunnels.h | 2 +-
net/ipv4/ip_tunnel.c | 11 +++++------
3 files changed, 22 insertions(+), 25 deletions(-)
diff --git a/include/net/gro_cells.h b/include/net/gro_cells.h
index 0f712c0..b1aeea1 100644
--- a/include/net/gro_cells.h
+++ b/include/net/gro_cells.h
@@ -10,21 +10,18 @@ struct gro_cell {
struct napi_struct napi;
};
-struct gro_cells {
- struct gro_cell __percpu *cells;
-};
-
-static inline void gro_cells_receive(struct gro_cells *gcells, struct sk_buff *skb)
+static inline void gro_cells_receive(struct gro_cell __percpu *gcells,
+ struct sk_buff *skb)
{
struct gro_cell *cell;
struct net_device *dev = skb->dev;
- if (!gcells->cells || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
+ if (!gcells || skb_cloned(skb) || !(dev->features & NETIF_F_GRO)) {
netif_rx(skb);
return;
}
- cell = this_cpu_ptr(gcells->cells);
+ cell = this_cpu_ptr(gcells);
if (skb_queue_len(&cell->napi_skbs) > netdev_max_backlog) {
atomic_long_inc(&dev->rx_dropped);
@@ -66,37 +63,38 @@ static inline int gro_cell_poll(struct napi_struct *napi, int budget)
return work_done;
}
-static inline int gro_cells_init(struct gro_cells *gcells, struct net_device *dev)
+static inline struct gro_cell __percpu *
+gro_cell_alloc_percpu(struct net_device *dev)
{
+ struct gro_cell __percpu *gcells;
int i;
- gcells->cells = alloc_percpu(struct gro_cell);
- if (!gcells->cells)
- return -ENOMEM;
+ gcells = alloc_percpu(struct gro_cell);
+ if (!gcells)
+ return ERR_PTR(-ENOMEM);
for_each_possible_cpu(i) {
- struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
+ struct gro_cell *cell = per_cpu_ptr(gcells, i);
skb_queue_head_init(&cell->napi_skbs);
netif_napi_add(dev, &cell->napi, gro_cell_poll, 64);
napi_enable(&cell->napi);
}
- return 0;
+ return gcells;
}
-static inline void gro_cells_destroy(struct gro_cells *gcells)
+static inline void gro_cell_free_percpu(struct gro_cell __percpu *gcells)
{
int i;
- if (!gcells->cells)
+ if (IS_ERR_OR_NULL(gcells))
return;
for_each_possible_cpu(i) {
- struct gro_cell *cell = per_cpu_ptr(gcells->cells, i);
+ struct gro_cell *cell = per_cpu_ptr(gcells, i);
netif_napi_del(&cell->napi);
skb_queue_purge(&cell->napi_skbs);
}
- free_percpu(gcells->cells);
- gcells->cells = NULL;
+ free_percpu(gcells);
}
#endif
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 25a59eb..e406bc3 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -83,7 +83,7 @@ struct ip_tunnel {
struct ip_tunnel_prl_entry __rcu *prl; /* potential router list */
unsigned int prl_count; /* # of entries in PRL */
int ip_tnl_net_id;
- struct gro_cells gro_cells;
+ struct gro_cell __percpu *gro_cells;
};
#define TUNNEL_CSUM __cpu_to_be16(0x01)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index d3e4479..fcc92c0 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -479,7 +479,7 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
skb->dev = tunnel->dev;
}
- gro_cells_receive(&tunnel->gro_cells, skb);
+ gro_cells_receive(tunnel->gro_cells, skb);
return 0;
drop:
@@ -952,7 +952,7 @@ static void ip_tunnel_dev_free(struct net_device *dev)
{
struct ip_tunnel *tunnel = netdev_priv(dev);
- gro_cells_destroy(&tunnel->gro_cells);
+ gro_cell_free_percpu(tunnel->gro_cells);
free_percpu(tunnel->dst_cache);
free_percpu(dev->tstats);
free_netdev(dev);
@@ -1120,7 +1120,6 @@ int ip_tunnel_init(struct net_device *dev)
{
struct ip_tunnel *tunnel = netdev_priv(dev);
struct iphdr *iph = &tunnel->parms.iph;
- int err;
dev->destructor = ip_tunnel_dev_free;
dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
@@ -1133,11 +1132,11 @@ int ip_tunnel_init(struct net_device *dev)
return -ENOMEM;
}
- err = gro_cells_init(&tunnel->gro_cells, dev);
- if (err) {
+ tunnel->gro_cells = gro_cell_alloc_percpu(dev);
+ if (IS_ERR(tunnel->gro_cells)) {
free_percpu(tunnel->dst_cache);
free_percpu(dev->tstats);
- return err;
+ return PTR_ERR(tunnel->gro_cells);
}
tunnel->dev = dev;
--
1.8.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells
2015-01-13 23:42 ` [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells Martin KaFai Lau
@ 2015-01-14 22:53 ` Eric Dumazet
2015-01-15 18:39 ` Martin Lau
0 siblings, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2015-01-14 22:53 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, kernel-team
On Tue, 2015-01-13 at 15:42 -0800, Martin KaFai Lau wrote:
> After adding percpu gro_cells, struct gro_cells can be removed.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
> include/net/gro_cells.h | 34 ++++++++++++++++------------------
> include/net/ip_tunnels.h | 2 +-
> net/ipv4/ip_tunnel.c | 11 +++++------
> 3 files changed, 22 insertions(+), 25 deletions(-)
>
> diff --git a/include/net/gro_cells.h b/include/net/gro_cells.h
> index 0f712c0..b1aeea1 100644
> --- a/include/net/gro_cells.h
> +++ b/include/net/gro_cells.h
> @@ -10,21 +10,18 @@ struct gro_cell {
> struct napi_struct napi;
> };
>
> -struct gro_cells {
> - struct gro_cell __percpu *cells;
> -};
> -
This seems a lot of code churn for no runtime difference ?
If we ever want to add an additional 'field', we likely have to revert
this patch.
-static inline void gro_cells_destroy(struct gro_cells *gcells)
+static inline void gro_cell_free_percpu(struct gro_cell __percpu *gcells)
{
int i;
- if (!gcells->cells)
+ if (IS_ERR_OR_NULL(gcells))
return;
For example, I have no idea why this part is needed.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC v2 net-next 1/2] ip_tunnel: Create percpu gro_cell
2015-01-13 23:42 ` [PATCH RFC v2 net-next 1/2] " Martin KaFai Lau
@ 2015-01-15 0:14 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2015-01-15 0:14 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev, kernel-team
On Tue, 2015-01-13 at 15:42 -0800, Martin KaFai Lau wrote:
> In the ipip tunnel, the skb->queue_mapping is lost in ipip_rcv().
> All skb will be queued to the same cell->napi_skbs. The
> gro_cell_poll is pinned to one core under load. In production traffic,
> we also see severe rx_dropped in the tunl iface and it is probably due to
> this limit: skb_queue_len(&cell->napi_skbs) > netdev_max_backlog.
>
> This patch is trying to alloc_percpu(struct gro_cell) and schedule
> gro_cell_poll to process the skb in the same core.
>
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells
2015-01-14 22:53 ` Eric Dumazet
@ 2015-01-15 18:39 ` Martin Lau
2015-01-15 19:24 ` Eric Dumazet
0 siblings, 1 reply; 7+ messages in thread
From: Martin Lau @ 2015-01-15 18:39 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, kernel-team
On Wed, Jan 14, 2015 at 02:53:59PM -0800, Eric Dumazet wrote:
> On Tue, 2015-01-13 at 15:42 -0800, Martin KaFai Lau wrote:
> > After adding percpu gro_cells, struct gro_cells can be removed.
> >
> > Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> > ---
> > include/net/gro_cells.h | 34 ++++++++++++++++------------------
> > include/net/ip_tunnels.h | 2 +-
> > net/ipv4/ip_tunnel.c | 11 +++++------
> > 3 files changed, 22 insertions(+), 25 deletions(-)
> >
> > diff --git a/include/net/gro_cells.h b/include/net/gro_cells.h
> > index 0f712c0..b1aeea1 100644
> > --- a/include/net/gro_cells.h
> > +++ b/include/net/gro_cells.h
> > @@ -10,21 +10,18 @@ struct gro_cell {
> > struct napi_struct napi;
> > };
> >
> > -struct gro_cells {
> > - struct gro_cell __percpu *cells;
> > -};
> > -
>
> This seems a lot of code churn for no runtime difference ?
>
> If we ever want to add an additional 'field', we likely have to revert
> this patch.
It seems cleaning up an one item struct is unnecessary. I
will repost without patch 2/2.
>
> -static inline void gro_cells_destroy(struct gro_cells *gcells)
> +static inline void gro_cell_free_percpu(struct gro_cell __percpu *gcells)
> {
> int i;
>
> - if (!gcells->cells)
> + if (IS_ERR_OR_NULL(gcells))
> return;
>
> For example, I have no idea why this part is needed.
For this change:
- err = gro_cells_init(&tunnel->gro_cells, dev);
- if (err) {
+ tunnel->gro_cells = gro_cell_alloc_percpu(dev);
+ if (IS_ERR(tunnel->gro_cells)) {
Thanks,
--Martin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells
2015-01-15 18:39 ` Martin Lau
@ 2015-01-15 19:24 ` Eric Dumazet
0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2015-01-15 19:24 UTC (permalink / raw)
To: Martin Lau; +Cc: netdev, kernel-team
On Thu, 2015-01-15 at 10:39 -0800, Martin Lau wrote:
> >
> > -static inline void gro_cells_destroy(struct gro_cells *gcells)
> > +static inline void gro_cell_free_percpu(struct gro_cell __percpu *gcells)
> > {
> > int i;
> >
> > - if (!gcells->cells)
> > + if (IS_ERR_OR_NULL(gcells))
> > return;
> >
> > For example, I have no idea why this part is needed.
> For this change:
> - err = gro_cells_init(&tunnel->gro_cells, dev);
> - if (err) {
> + tunnel->gro_cells = gro_cell_alloc_percpu(dev);
> + if (IS_ERR(tunnel->gro_cells)) {
That is bad. See David Miller recent mail about this kind of construct.
Current code is better : Do not store an error code in structure, but
rather use a local variable.
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-15 19:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-13 23:42 [PATCH RFC v2 net-next 0/2] ip_tunnel: Create percpu gro_cell Martin KaFai Lau
2015-01-13 23:42 ` [PATCH RFC v2 net-next 1/2] " Martin KaFai Lau
2015-01-15 0:14 ` Eric Dumazet
2015-01-13 23:42 ` [PATCH RFC v2 net-next 2/2] ip_tunnel: Remove struct gro_cells Martin KaFai Lau
2015-01-14 22:53 ` Eric Dumazet
2015-01-15 18:39 ` Martin Lau
2015-01-15 19:24 ` Eric Dumazet
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.