* Re: Question about ip_defrag
[not found] <4F88C5DDA1E80143B232E89585ACE27D018F07E2@DGGEMA502-MBX.china.huawei.com>
@ 2017-08-24 13:53 ` Jesper Dangaard Brouer
[not found] ` <4F88C5DDA1E80143B232E89585ACE27D018F0AE1@DGGEMA502-MBX.china.huawei.com>
0 siblings, 1 reply; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2017-08-24 13:53 UTC (permalink / raw)
To: liujian (CE)
Cc: davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev, brouer
On Thu, 24 Aug 2017 13:15:33 +0000 "liujian (CE)" <liujian56@huawei.com> wrote:
> Hello,
>
> With below patch we met one issue.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.13-rc6&id=6d7b857d541e
>
> the issue:
> Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> At this moment,sum_frag_mem_limit is about 10K.
> and my test machine's cpu num is 64.
>
> Can i only change frag_mem_limit to sum_ frag_mem_limit?
>
>
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index 96e95e8..f09c00b 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -120,7 +120,7 @@ static void inet_frag_secret_rebuild(struct inet_frags *f)
> static bool inet_fragq_should_evict(const struct inet_frag_queue *q)
> {
> return q->net->low_thresh == 0 ||
> - frag_mem_limit(q->net) >= q->net->low_thresh;
> + sum_frag_mem_limit(q->net) >= q->net->low_thresh;
> }
>
> static unsigned int
> @@ -355,7 +355,7 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
> {
> struct inet_frag_queue *q;
>
> - if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh) {
> + if (!nf->high_thresh || sum_frag_mem_limit(nf) > nf->high_thresh) {
> inet_frag_schedule_worker(f);
> return NULL;
> }
> @@ -396,7 +396,7 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> struct inet_frag_queue *q;
> int depth = 0;
>
> - if (frag_mem_limit(nf) > nf->low_thresh)
> + if (sum_frag_mem_limit(nf) > nf->low_thresh)
> inet_frag_schedule_worker(f);
>
> hash &= (INETFRAGS_HASHSZ - 1);
> --
>
> Thank you for your time.
What kernel version have you seen this issue with?
As far as I remember, this issue have been fixed before...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
[not found] ` <4F88C5DDA1E80143B232E89585ACE27D018F0AE1@DGGEMA502-MBX.china.huawei.com>
@ 2017-08-24 18:59 ` Jesper Dangaard Brouer
2017-08-25 1:33 ` liujian (CE)
2017-08-28 8:08 ` liujian (CE)
0 siblings, 2 replies; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2017-08-24 18:59 UTC (permalink / raw)
To: liujian (CE)
Cc: davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev, brouer
On Thu, 24 Aug 2017 16:04:41 +0000 "liujian (CE)" <liujian56@huawei.com> wrote:
> >What kernel version have you seen this issue with?
>
> 3.10，with some backport.
>
> >As far as I remember, this issue have been fixed before...
>
> which one patch? I didnot find out the patch:(
AFAIK it was some bugs in the percpu_counter code. If you need to
backport look at the git commits:
git log lib/percpu_counter.c include/linux/percpu_counter.h
Are you maintaining your own 3.10 kernel?
I know that for RHEL7 (also kernel 3.10) we backported the
percpu_counter fixes...
--Jesper
> 发件人： Jesper Dangaard Brouer
> 收件人： liujian (CE)<liujian56@huawei.com<mailto:liujian56@huawei.com>>
> 抄送： davem@davemloft.net<mailto:davem@davemloft.net>;kuznet@ms2.inr.ac.ru<mailto:kuznet@ms2.inr.ac.ru>;yoshfuji@linux-ipv6.org<mailto:yoshfuji@linux-ipv6.org>;elena.reshetova@intel.com<mailto:elena.reshetova@intel.com>;edumazet@google.com<mailto:edumazet@google.com>;netdev@vger.kernel.org<mailto:netdev@vger.kernel.org>;brouer@redhat.com<mailto:brouer@redhat.com>
> 主题： Re: Question about ip_defrag
> 时间： 2017-08-24 21:53:17
>
>
> On Thu, 24 Aug 2017 13:15:33 +0000 "liujian (CE)" <liujian56@huawei.com> wrote:
> > Hello,
> >
> > With below patch we met one issue.
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.13-rc6&id=6d7b857d541e
> >
> > the issue:
> > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > At this moment,sum_frag_mem_limit is about 10K.
> > and my test machine's cpu num is 64.
> >
> > Can i only change frag_mem_limit to sum_ frag_mem_limit?
> >
> >
> > diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> > index 96e95e8..f09c00b 100644
> > --- a/net/ipv4/inet_fragment.c
> > +++ b/net/ipv4/inet_fragment.c
> > @@ -120,7 +120,7 @@ static void inet_frag_secret_rebuild(struct inet_frags *f)
> > static bool inet_fragq_should_evict(const struct inet_frag_queue *q)
> > {
> > return q->net->low_thresh == 0 ||
> > - frag_mem_limit(q->net) >= q->net->low_thresh;
> > + sum_frag_mem_limit(q->net) >= q->net->low_thresh;
> > }
> >
> > static unsigned int
> > @@ -355,7 +355,7 @@ static struct inet_frag_queue *inet_frag_alloc(struct netns_frags *nf,
> > {
> > struct inet_frag_queue *q;
> >
> > - if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh) {
> > + if (!nf->high_thresh || sum_frag_mem_limit(nf) > nf->high_thresh) {
> > inet_frag_schedule_worker(f);
> > return NULL;
> > }
> > @@ -396,7 +396,7 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
> > struct inet_frag_queue *q;
> > int depth = 0;
> >
> > - if (frag_mem_limit(nf) > nf->low_thresh)
> > + if (sum_frag_mem_limit(nf) > nf->low_thresh)
> > inet_frag_schedule_worker(f);
> >
> > hash &= (INETFRAGS_HASHSZ - 1);
> > --
> >
> > Thank you for your time.
>
> What kernel version have you seen this issue with?
>
> As far as I remember, this issue have been fixed before...
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-24 18:59 ` Jesper Dangaard Brouer
@ 2017-08-25 1:33 ` liujian (CE)
2017-08-28 8:08 ` liujian (CE)
1 sibling, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2017-08-25 1:33 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev,
Wangkefeng (Kevin), weiyongjun (A)
> -----Original Message-----
> From: Jesper Dangaard Brouer [mailto:brouer@redhat.com]
> Sent: Friday, August 25, 2017 2:59 AM
> To: liujian (CE)
> Cc: davem@davemloft.net; kuznet@ms2.inr.ac.ru; yoshfuji@linux-ipv6.org;
> elena.reshetova@intel.com; edumazet@google.com; netdev@vger.kernel.org;
> brouer@redhat.com
> Subject: Re: Question about ip_defrag
>
>
> On Thu, 24 Aug 2017 16:04:41 +0000 "liujian (CE)" <liujian56@huawei.com>
> wrote:
>
> > >What kernel version have you seen this issue with?
> >
> > 3.10，with some backport.
> >
> > >As far as I remember, this issue have been fixed before...
> >
> > which one patch? I didnot find out the patch:(
>
> AFAIK it was some bugs in the percpu_counter code. If you need to backport
> look at the git commits:
>
> git log lib/percpu_counter.c include/linux/percpu_counter.h
>
> Are you maintaining your own 3.10 kernel?
>
> I know that for RHEL7 (also kernel 3.10) we backported the percpu_counter
> fixes...
>
Could you tell me which one patch? we have backported most of the two files's change.
Thank you ~
> --Jesper
>
>
> > 发件人： Jesper Dangaard Brouer
> > 收件人： liujian
> (CE)<liujian56@huawei.com<mailto:liujian56@huawei.com>>
> > 抄送：
> >
> davem@davemloft.net<mailto:davem@davemloft.net>;kuznet@ms2.inr.ac.ru
> <m
> > ailto:kuznet@ms2.inr.ac.ru>;yoshfuji@linux-ipv6.org<mailto:yoshfuji@li
> > nux-ipv6.org>;elena.reshetova@intel.com<mailto:elena.reshetova@intel.c
> >
> om>;edumazet@google.com<mailto:edumazet@google.com>;netdev@vger.k
> ernel
> > .org<mailto:netdev@vger.kernel.org>;brouer@redhat.com<mailto:brouer@r
> e
> > dhat.com>
> > 主题： Re: Question about ip_defrag
> > 时间： 2017-08-24 21:53:17
> >
> >
> > On Thu, 24 Aug 2017 13:15:33 +0000 "liujian (CE)" <liujian56@huawei.com>
> wrote:
> > > Hello,
> > >
> > > With below patch we met one issue.
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/c
> > > ommit/?h=v4.13-rc6&id=6d7b857d541e
> > >
> > > the issue:
> > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > At this moment,sum_frag_mem_limit is about 10K.
> > > and my test machine's cpu num is 64.
> > >
> > > Can i only change frag_mem_limit to sum_ frag_mem_limit?
> > >
> > >
> > > diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> > > index 96e95e8..f09c00b 100644
> > > --- a/net/ipv4/inet_fragment.c
> > > +++ b/net/ipv4/inet_fragment.c
> > > @@ -120,7 +120,7 @@ static void inet_frag_secret_rebuild(struct
> > > inet_frags *f) static bool inet_fragq_should_evict(const struct
> > > inet_frag_queue *q) {
> > > return q->net->low_thresh == 0 ||
> > > - frag_mem_limit(q->net) >= q->net->low_thresh;
> > > + sum_frag_mem_limit(q->net) >= q->net->low_thresh;
> > > }
> > >
> > > static unsigned int
> > > @@ -355,7 +355,7 @@ static struct inet_frag_queue
> > > *inet_frag_alloc(struct netns_frags *nf, {
> > > struct inet_frag_queue *q;
> > >
> > > - if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh) {
> > > + if (!nf->high_thresh || sum_frag_mem_limit(nf) >
> > > + nf->high_thresh) {
> > > inet_frag_schedule_worker(f);
> > > return NULL;
> > > }
> > > @@ -396,7 +396,7 @@ struct inet_frag_queue *inet_frag_find(struct
> netns_frags *nf,
> > > struct inet_frag_queue *q;
> > > int depth = 0;
> > >
> > > - if (frag_mem_limit(nf) > nf->low_thresh)
> > > + if (sum_frag_mem_limit(nf) > nf->low_thresh)
> > > inet_frag_schedule_worker(f);
> > >
> > > hash &= (INETFRAGS_HASHSZ - 1);
> > > --
> > >
> > > Thank you for your time.
> >
> > What kernel version have you seen this issue with?
> >
> > As far as I remember, this issue have been fixed before...
> >
> > --
> > Best regards,
> > Jesper Dangaard Brouer
> > MSc.CS, Principal Kernel Engineer at Red Hat
> > LinkedIn: http://www.linkedin.com/in/brouer
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-24 18:59 ` Jesper Dangaard Brouer
2017-08-25 1:33 ` liujian (CE)
@ 2017-08-28 8:08 ` liujian (CE)
2017-08-28 14:00 ` Florian Westphal
1 sibling, 1 reply; 15+ messages in thread
From: liujian (CE) @ 2017-08-28 8:08 UTC (permalink / raw)
To: liujian (CE), Jesper Dangaard Brouer
Cc: davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev,
Wangkefeng (Kevin), weiyongjun (A)
Hi
I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > > > the issue:
> > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > At this moment,sum_frag_mem_limit is about 10K.
So should we change ipfrag high/low thresh to a reasonable value ?
And if it is, is there a standard to change the value?
root@RH8100-V3:/proc/net# cat sockstat
sockets: used 1485
TCP: inuse 4 orphan 0 tw 0 alloc 5 mem 1
UDP: inuse 203 mem 201
UDPLITE: inuse 0
RAW: inuse 0
FRAG: inuse 1 memory 16048, 3156696.
root@RH8100-V3:/proc/net#
In order to print frag_mem_limit, change the code as below:
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 43eb6567..38bfb20 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -73,7 +73,7 @@ static int sockstat_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, "RAW: inuse %d\n",
sock_prot_inuse_get(net, &raw_prot));
frag_mem = ip_frag_mem(net);
- seq_printf(seq, "FRAG: inuse %u memory %u\n", !!frag_mem, frag_mem);
+ seq_printf(seq, "FRAG: inuse %u memory %u, %u.\n", !!frag_mem, frag_mem, frag_mem_limit(&net->ipv4.frags));
return 0;
}
Best Regards,
liujian
> -----Original Message-----
> From: liujian (CE)
> Sent: Friday, August 25, 2017 9:33 AM
> To: 'Jesper Dangaard Brouer'
> Cc: davem@davemloft.net; kuznet@ms2.inr.ac.ru; yoshfuji@linux-ipv6.org;
> elena.reshetova@intel.com; edumazet@google.com; netdev@vger.kernel.org;
> Wangkefeng (Kevin); weiyongjun (A)
> Subject: RE: Question about ip_defrag
>
>
> > -----Original Message-----
> > From: Jesper Dangaard Brouer [mailto:brouer@redhat.com]
> > Sent: Friday, August 25, 2017 2:59 AM
> > To: liujian (CE)
> > Cc: davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> > yoshfuji@linux-ipv6.org; elena.reshetova@intel.com;
> > edumazet@google.com; netdev@vger.kernel.org; brouer@redhat.com
> > Subject: Re: Question about ip_defrag
> >
> >
> > On Thu, 24 Aug 2017 16:04:41 +0000 "liujian (CE)"
> > <liujian56@huawei.com>
> > wrote:
> >
> > > >What kernel version have you seen this issue with?
> > >
> > > 3.10，with some backport.
> > >
> > > >As far as I remember, this issue have been fixed before...
> > >
> > > which one patch? I didnot find out the patch:(
> >
> > AFAIK it was some bugs in the percpu_counter code. If you need to
> > backport look at the git commits:
> >
> > git log lib/percpu_counter.c include/linux/percpu_counter.h
> >
> > Are you maintaining your own 3.10 kernel?
> >
> > I know that for RHEL7 (also kernel 3.10) we backported the
> > percpu_counter fixes...
> >
> Could you tell me which one patch? we have backported most of the two
> files's change.
> Thank you ~
>
>
> > --Jesper
> >
> >
> > > 发件人： Jesper Dangaard Brouer
> > > 收件人： liujian
> > (CE)<liujian56@huawei.com<mailto:liujian56@huawei.com>>
> > > 抄送：
> > >
> >
> davem@davemloft.net<mailto:davem@davemloft.net>;kuznet@ms2.inr.ac.ru
> > <m
> > > ailto:kuznet@ms2.inr.ac.ru>;yoshfuji@linux-ipv6.org<mailto:yoshfuji@
> > > li
> > > nux-ipv6.org>;elena.reshetova@intel.com<mailto:elena.reshetova@intel
> > > .c
> > >
> >
> om>;edumazet@google.com<mailto:edumazet@google.com>;netdev@vger.k
> > ernel
> > > .org<mailto:netdev@vger.kernel.org>;brouer@redhat.com<mailto:brouer
> @
> > > r
> > e
> > > dhat.com>
> > > 主题： Re: Question about ip_defrag
> > > 时间： 2017-08-24 21:53:17
> > >
> > >
> > > On Thu, 24 Aug 2017 13:15:33 +0000 "liujian (CE)"
> > > <liujian56@huawei.com>
> > wrote:
> > > > Hello,
> > > >
> > > > With below patch we met one issue.
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > > > /c ommit/?h=v4.13-rc6&id=6d7b857d541e
> > > >
> > > > the issue:
> > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > At this moment,sum_frag_mem_limit is about 10K.
> > > > and my test machine's cpu num is 64.
> > > >
> > > > Can i only change frag_mem_limit to sum_ frag_mem_limit?
> > > >
> > > >
> > > > diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> > > > index 96e95e8..f09c00b 100644
> > > > --- a/net/ipv4/inet_fragment.c
> > > > +++ b/net/ipv4/inet_fragment.c
> > > > @@ -120,7 +120,7 @@ static void inet_frag_secret_rebuild(struct
> > > > inet_frags *f) static bool inet_fragq_should_evict(const struct
> > > > inet_frag_queue *q) {
> > > > return q->net->low_thresh == 0 ||
> > > > - frag_mem_limit(q->net) >= q->net->low_thresh;
> > > > + sum_frag_mem_limit(q->net) >= q->net->low_thresh;
> > > > }
> > > >
> > > > static unsigned int
> > > > @@ -355,7 +355,7 @@ static struct inet_frag_queue
> > > > *inet_frag_alloc(struct netns_frags *nf, {
> > > > struct inet_frag_queue *q;
> > > >
> > > > - if (!nf->high_thresh || frag_mem_limit(nf) > nf->high_thresh) {
> > > > + if (!nf->high_thresh || sum_frag_mem_limit(nf) >
> > > > + nf->high_thresh) {
> > > > inet_frag_schedule_worker(f);
> > > > return NULL;
> > > > }
> > > > @@ -396,7 +396,7 @@ struct inet_frag_queue *inet_frag_find(struct
> > netns_frags *nf,
> > > > struct inet_frag_queue *q;
> > > > int depth = 0;
> > > >
> > > > - if (frag_mem_limit(nf) > nf->low_thresh)
> > > > + if (sum_frag_mem_limit(nf) > nf->low_thresh)
> > > > inet_frag_schedule_worker(f);
> > > >
> > > > hash &= (INETFRAGS_HASHSZ - 1);
> > > > --
> > > >
> > > > Thank you for your time.
> > >
> > > What kernel version have you seen this issue with?
> > >
> > > As far as I remember, this issue have been fixed before...
> > >
> > > --
> > > Best regards,
> > > Jesper Dangaard Brouer
> > > MSc.CS, Principal Kernel Engineer at Red Hat
> > > LinkedIn: http://www.linkedin.com/in/brouer
> >
> >
> >
> > --
> > Best regards,
> > Jesper Dangaard Brouer
> > MSc.CS, Principal Kernel Engineer at Red Hat
> > LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-28 8:08 ` liujian (CE)
@ 2017-08-28 14:00 ` Florian Westphal
2017-08-29 7:20 ` Jesper Dangaard Brouer
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Florian Westphal @ 2017-08-28 14:00 UTC (permalink / raw)
To: liujian (CE)
Cc: Jesper Dangaard Brouer, davem, kuznet, yoshfuji, elena.reshetova,
edumazet, netdev, Wangkefeng (Kevin), weiyongjun (A)
liujian (CE) <liujian56@huawei.com> wrote:
> Hi
>
> I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
>
> > > > > the issue:
> > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > At this moment,sum_frag_mem_limit is about 10K.
>
> So should we change ipfrag high/low thresh to a reasonable value ?
> And if it is, is there a standard to change the value?
Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
about so with 64 cpus that is ~8 mbyte.
possible solutions:
1. reduce frag_percpu_counter_batch to 16k or so
2. make both low and high thresh depend on NR_CPUS
liujian, does this change help in any way?
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -123,6 +123,17 @@ static bool inet_fragq_should_evict(const struct inet_frag_queue *q)
frag_mem_limit(q->net) >= q->net->low_thresh;
}
+/* ->mem batch size is huge, this can cause severe discrepancies
+ * between actual value (sum of pcpu values) and the global estimate.
+ *
+ * Use a smaller batch to give an opportunity for the global estimate
+ * to more accurately reflect current state.
+ */
+static void update_frag_mem_limit(struct netns_frags *nf, unsigned int batch)
+{
+ percpu_counter_add_batch(&nf->mem, 0, batch);
+}
+
static unsigned int
inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
{
@@ -146,8 +157,12 @@ inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
spin_unlock(&hb->chain_lock);
- hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
+ hlist_for_each_entry_safe(fq, n, &expired, list_evictor) {
+ struct netns_frags *nf = fq->net;
+
f->frag_expire((unsigned long) fq);
+ update_frag_mem_limit(nf, 1);
+ }
return evicted;
}
@@ -396,8 +411,10 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
struct inet_frag_queue *q;
int depth = 0;
- if (frag_mem_limit(nf) > nf->low_thresh)
+ if (frag_mem_limit(nf) > nf->low_thresh) {
inet_frag_schedule_worker(f);
+ update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
+ }
hash &= (INETFRAGS_HASHSZ - 1);
hb = &f->hash[hash];
@@ -416,6 +433,8 @@ struct inet_frag_queue *inet_frag_find(struct netns_frags *nf,
if (depth <= INETFRAGS_MAXDEPTH)
return inet_frag_create(nf, f, key);
+ update_frag_mem_limit(nf, 1);
+
if (inet_frag_may_rebuild(f)) {
if (!f->rebuild)
f->rebuild = true;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-28 14:00 ` Florian Westphal
@ 2017-08-29 7:20 ` Jesper Dangaard Brouer
2017-08-29 7:44 ` liujian (CE)
2017-08-29 7:53 ` Florian Westphal
2017-08-29 7:40 ` liujian (CE)
2017-08-29 13:01 ` liujian (CE)
2 siblings, 2 replies; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2017-08-29 7:20 UTC (permalink / raw)
To: Florian Westphal
Cc: liujian (CE),
davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev,
Wangkefeng (Kevin), weiyongjun (A),
brouer
On Mon, 28 Aug 2017 16:00:32 +0200
Florian Westphal <fw@strlen.de> wrote:
> liujian (CE) <liujian56@huawei.com> wrote:
> > Hi
> >
> > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> >
> > > > > > the issue:
> > > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > > At this moment,sum_frag_mem_limit is about 10K.
> >
> > So should we change ipfrag high/low thresh to a reasonable value ?
> > And if it is, is there a standard to change the value?
>
> Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> about so with 64 cpus that is ~8 mbyte.
>
> possible solutions:
> 1. reduce frag_percpu_counter_batch to 16k or so
> 2. make both low and high thresh depend on NR_CPUS
To me it looks like we/I have been using the wrong API for comparing
against percpu_counters. I guess we should have used __percpu_counter_compare().
/*
* Compare counter against given value.
* Return 1 if greater, 0 if equal and -1 if less
*/
int __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch)
{
s64 count;
count = percpu_counter_read(fbc);
/* Check to see if rough count will be sufficient for comparison */
if (abs(count - rhs) > (batch * num_online_cpus())) {
if (count > rhs)
return 1;
else
return -1;
}
/* Need to use precise count */
count = percpu_counter_sum(fbc);
if (count > rhs)
return 1;
else if (count < rhs)
return -1;
else
return 0;
}
EXPORT_SYMBOL(__percpu_counter_compare);
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-28 14:00 ` Florian Westphal
2017-08-29 7:20 ` Jesper Dangaard Brouer
@ 2017-08-29 7:40 ` liujian (CE)
2017-08-29 13:01 ` liujian (CE)
2 siblings, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2017-08-29 7:40 UTC (permalink / raw)
To: Florian Westphal
Cc: Jesper Dangaard Brouer, davem, kuznet, yoshfuji, elena.reshetova,
edumazet, netdev, Wangkefeng (Kevin), weiyongjun (A)
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Florian Westphal
> Sent: Monday, August 28, 2017 10:01 PM
> To: liujian (CE)
> Cc: Jesper Dangaard Brouer; davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> yoshfuji@linux-ipv6.org; elena.reshetova@intel.com; edumazet@google.com;
> netdev@vger.kernel.org; Wangkefeng (Kevin); weiyongjun (A)
> Subject: Re: Question about ip_defrag
>
> liujian (CE) <liujian56@huawei.com> wrote:
> > Hi
> >
> > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in
> lib/percpu_counter.c and include/linux/percpu_counter.h.
> > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> >
> > > > > > the issue:
> > > > > > Ip_defrag fail caused by frag_mem_limit reached
> 4M(frags.high_thresh).
> > > > > > At this moment,sum_frag_mem_limit is about 10K.
> >
> > So should we change ipfrag high/low thresh to a reasonable value ?
> > And if it is, is there a standard to change the value?
>
> Each cpu can have frag_percpu_counter_batch bytes rest doesn't know about
> so with 64 cpus that is ~8 mbyte.
>
> possible solutions:
> 1. reduce frag_percpu_counter_batch to 16k or so 2. make both low and high
> thresh depend on NR_CPUS
>
Thank you for your reply.
> liujian, does this change help in any way?
I will have a try.
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -123,6 +123,17 @@ static bool inet_fragq_should_evict(const struct
> inet_frag_queue *q)
> frag_mem_limit(q->net) >= q->net->low_thresh; }
>
> +/* ->mem batch size is huge, this can cause severe discrepancies
> + * between actual value (sum of pcpu values) and the global estimate.
> + *
> + * Use a smaller batch to give an opportunity for the global estimate
> + * to more accurately reflect current state.
> + */
> +static void update_frag_mem_limit(struct netns_frags *nf, unsigned int
> +batch) {
> + percpu_counter_add_batch(&nf->mem, 0, batch); }
> +
> static unsigned int
> inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb) { @@
> -146,8 +157,12 @@ inet_evict_bucket(struct inet_frags *f, struct
> inet_frag_bucket *hb)
>
> spin_unlock(&hb->chain_lock);
>
> - hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
> + hlist_for_each_entry_safe(fq, n, &expired, list_evictor) {
> + struct netns_frags *nf = fq->net;
> +
> f->frag_expire((unsigned long) fq);
> + update_frag_mem_limit(nf, 1);
> + }
>
> return evicted;
> }
> @@ -396,8 +411,10 @@ struct inet_frag_queue *inet_frag_find(struct
> netns_frags *nf,
> struct inet_frag_queue *q;
> int depth = 0;
>
> - if (frag_mem_limit(nf) > nf->low_thresh)
> + if (frag_mem_limit(nf) > nf->low_thresh) {
> inet_frag_schedule_worker(f);
> + update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
> + }
>
> hash &= (INETFRAGS_HASHSZ - 1);
> hb = &f->hash[hash];
> @@ -416,6 +433,8 @@ struct inet_frag_queue *inet_frag_find(struct
> netns_frags *nf,
> if (depth <= INETFRAGS_MAXDEPTH)
> return inet_frag_create(nf, f, key);
>
> + update_frag_mem_limit(nf, 1);
> +
> if (inet_frag_may_rebuild(f)) {
> if (!f->rebuild)
> f->rebuild = true;
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-29 7:20 ` Jesper Dangaard Brouer
@ 2017-08-29 7:44 ` liujian (CE)
2017-08-29 7:53 ` Florian Westphal
1 sibling, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2017-08-29 7:44 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Florian Westphal
Cc: davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev,
Wangkefeng (Kevin), weiyongjun (A)
> -----Original Message-----
> From: Jesper Dangaard Brouer [mailto:brouer@redhat.com]
> Sent: Tuesday, August 29, 2017 3:20 PM
> To: Florian Westphal
> Cc: liujian (CE); davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> yoshfuji@linux-ipv6.org; elena.reshetova@intel.com; edumazet@google.com;
> netdev@vger.kernel.org; Wangkefeng (Kevin); weiyongjun (A);
> brouer@redhat.com
> Subject: Re: Question about ip_defrag
>
> On Mon, 28 Aug 2017 16:00:32 +0200
> Florian Westphal <fw@strlen.de> wrote:
>
> > liujian (CE) <liujian56@huawei.com> wrote:
> > > Hi
> > >
> > > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in
> lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > >
> > > > > > > the issue:
> > > > > > > Ip_defrag fail caused by frag_mem_limit reached
> 4M(frags.high_thresh).
> > > > > > > At this moment,sum_frag_mem_limit is about 10K.
> > >
> > > So should we change ipfrag high/low thresh to a reasonable value ?
> > > And if it is, is there a standard to change the value?
> >
> > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > about so with 64 cpus that is ~8 mbyte.
> >
> > possible solutions:
> > 1. reduce frag_percpu_counter_batch to 16k or so 2. make both low and
> > high thresh depend on NR_CPUS
>
> To me it looks like we/I have been using the wrong API for comparing against
> percpu_counters. I guess we should have used
> __percpu_counter_compare().
Are you means?
Change
if (frag_mem_limit(nf) > nf->low_thresh)
to
__percpu_counter_compare(&nf->mem, nf->low_thresh, frag_percpu_counter_batch)
> /*
> * Compare counter against given value.
> * Return 1 if greater, 0 if equal and -1 if less */ int
> __percpu_counter_compare(struct percpu_counter *fbc, s64 rhs, s32 batch) {
> s64 count;
>
> count = percpu_counter_read(fbc);
> /* Check to see if rough count will be sufficient for comparison */
> if (abs(count - rhs) > (batch * num_online_cpus())) {
> if (count > rhs)
> return 1;
> else
> return -1;
> }
> /* Need to use precise count */
> count = percpu_counter_sum(fbc);
> if (count > rhs)
> return 1;
> else if (count < rhs)
> return -1;
> else
> return 0;
> }
> EXPORT_SYMBOL(__percpu_counter_compare);
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-29 7:20 ` Jesper Dangaard Brouer
2017-08-29 7:44 ` liujian (CE)
@ 2017-08-29 7:53 ` Florian Westphal
2017-08-30 10:58 ` Jesper Dangaard Brouer
1 sibling, 1 reply; 15+ messages in thread
From: Florian Westphal @ 2017-08-29 7:53 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Florian Westphal, liujian (CE),
davem, kuznet, yoshfuji, elena.reshetova, edumazet, netdev,
Wangkefeng (Kevin), weiyongjun (A)
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> On Mon, 28 Aug 2017 16:00:32 +0200
> Florian Westphal <fw@strlen.de> wrote:
>
> > liujian (CE) <liujian56@huawei.com> wrote:
> > > Hi
> > >
> > > I checked our 3.10 kernel, we had backported all percpu_counter bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > >
> > > > > > > the issue:
> > > > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > > > At this moment,sum_frag_mem_limit is about 10K.
> > >
> > > So should we change ipfrag high/low thresh to a reasonable value ?
> > > And if it is, is there a standard to change the value?
> >
> > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > about so with 64 cpus that is ~8 mbyte.
> >
> > possible solutions:
> > 1. reduce frag_percpu_counter_batch to 16k or so
> > 2. make both low and high thresh depend on NR_CPUS
I take 2) back. Its wrong to do this, for large NR_CPU values it
would even overflow.
> To me it looks like we/I have been using the wrong API for comparing
> against percpu_counters. I guess we should have used __percpu_counter_compare().
Are you sure? For liujian use case (64 cores) it looks like we would
always fall through to percpu_counter_sum() so we eat spinlock_irqsave
cost for all compares.
Before we entertain this we should consider reducing frag_percpu_counter_batch
to a smaller value.
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-28 14:00 ` Florian Westphal
2017-08-29 7:20 ` Jesper Dangaard Brouer
2017-08-29 7:40 ` liujian (CE)
@ 2017-08-29 13:01 ` liujian (CE)
2017-08-29 13:46 ` Florian Westphal
2 siblings, 1 reply; 15+ messages in thread
From: liujian (CE) @ 2017-08-29 13:01 UTC (permalink / raw)
To: liujian (CE), Florian Westphal
Cc: Jesper Dangaard Brouer, davem, kuznet, yoshfuji, elena.reshetova,
edumazet, netdev, Wangkefeng (Kevin), weiyongjun (A)
Best Regards,
liujian
> -----Original Message-----
> From: liujian (CE)
> Sent: Tuesday, August 29, 2017 3:39 PM
> To: 'Florian Westphal'
> Cc: Jesper Dangaard Brouer; davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> yoshfuji@linux-ipv6.org; elena.reshetova@intel.com; edumazet@google.com;
> netdev@vger.kernel.org; Wangkefeng (Kevin); weiyongjun (A)
> Subject: RE: Question about ip_defrag
>
>
>
> > -----Original Message-----
> > From: netdev-owner@vger.kernel.org
> > [mailto:netdev-owner@vger.kernel.org]
> > On Behalf Of Florian Westphal
> > Sent: Monday, August 28, 2017 10:01 PM
> > To: liujian (CE)
> > Cc: Jesper Dangaard Brouer; davem@davemloft.net; kuznet@ms2.inr.ac.ru;
> > yoshfuji@linux-ipv6.org; elena.reshetova@intel.com;
> > edumazet@google.com; netdev@vger.kernel.org; Wangkefeng (Kevin);
> > weiyongjun (A)
> > Subject: Re: Question about ip_defrag
> >
> > liujian (CE) <liujian56@huawei.com> wrote:
> > > Hi
> > >
> > > I checked our 3.10 kernel, we had backported all percpu_counter bug
> > > fix in
> > lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > >
> > > > > > > the issue:
> > > > > > > Ip_defrag fail caused by frag_mem_limit reached
> > 4M(frags.high_thresh).
> > > > > > > At this moment,sum_frag_mem_limit is about 10K.
> > >
> > > So should we change ipfrag high/low thresh to a reasonable value ?
> > > And if it is, is there a standard to change the value?
> >
> > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > about so with 64 cpus that is ~8 mbyte.
> >
> > possible solutions:
> > 1. reduce frag_percpu_counter_batch to 16k or so 2. make both low and
> > high thresh depend on NR_CPUS
> >
> Thank you for your reply.
>
> > liujian, does this change help in any way?
>
> I will have a try.
Now, I have not the real environment.
I use iperf generate fragment packets;
and I always change NIC rx irq's affinity cpu, to make sure frag_mem_limit reach to thresh.
my test machine, CPU num is 384.
As above , test the patch , seemingly , there is no improving...
Check /proc/net/snmp, there is no significant difference.
maybe we should find a good test method!
root@RH8100-V3:/proc/net# cat sockstat
sockets: used 1386
TCP: inuse 3 orphan 0 tw 0 alloc 4 mem 1
UDP: inuse 44 mem 42
UDPLITE: inuse 0
RAW: inuse 0
FRAG: inuse 1 memory 34336, 3144424.
> > diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> > --- a/net/ipv4/inet_fragment.c
> > +++ b/net/ipv4/inet_fragment.c
> > @@ -123,6 +123,17 @@ static bool inet_fragq_should_evict(const struct
> > inet_frag_queue *q)
> > frag_mem_limit(q->net) >= q->net->low_thresh; }
> >
> > +/* ->mem batch size is huge, this can cause severe discrepancies
> > + * between actual value (sum of pcpu values) and the global estimate.
> > + *
> > + * Use a smaller batch to give an opportunity for the global estimate
> > + * to more accurately reflect current state.
> > + */
> > +static void update_frag_mem_limit(struct netns_frags *nf, unsigned
> > +int
> > +batch) {
> > + percpu_counter_add_batch(&nf->mem, 0, batch); }
> > +
> > static unsigned int
> > inet_evict_bucket(struct inet_frags *f, struct inet_frag_bucket *hb)
> > { @@
> > -146,8 +157,12 @@ inet_evict_bucket(struct inet_frags *f, struct
> > inet_frag_bucket *hb)
> >
> > spin_unlock(&hb->chain_lock);
> >
> > - hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
> > + hlist_for_each_entry_safe(fq, n, &expired, list_evictor) {
> > + struct netns_frags *nf = fq->net;
> > +
> > f->frag_expire((unsigned long) fq);
> > + update_frag_mem_limit(nf, 1);
>
> > + }
> >
> > return evicted;
> > }
> > @@ -396,8 +411,10 @@ struct inet_frag_queue *inet_frag_find(struct
> > netns_frags *nf,
> > struct inet_frag_queue *q;
> > int depth = 0;
> >
> > - if (frag_mem_limit(nf) > nf->low_thresh)
> > + if (frag_mem_limit(nf) > nf->low_thresh) {
> > inet_frag_schedule_worker(f);
> > + update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
> > + }
> >
> > hash &= (INETFRAGS_HASHSZ - 1);
> > hb = &f->hash[hash];
> > @@ -416,6 +433,8 @@ struct inet_frag_queue *inet_frag_find(struct
> > netns_frags *nf,
> > if (depth <= INETFRAGS_MAXDEPTH)
> > return inet_frag_create(nf, f, key);
> >
> > + update_frag_mem_limit(nf, 1);
> > +
> > if (inet_frag_may_rebuild(f)) {
> > if (!f->rebuild)
> > f->rebuild = true;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-29 13:01 ` liujian (CE)
@ 2017-08-29 13:46 ` Florian Westphal
2017-08-30 1:52 ` liujian (CE)
0 siblings, 1 reply; 15+ messages in thread
From: Florian Westphal @ 2017-08-29 13:46 UTC (permalink / raw)
To: liujian (CE)
Cc: Florian Westphal, Jesper Dangaard Brouer, netdev,
Wangkefeng (Kevin), weiyongjun (A)
liujian (CE) <liujian56@huawei.com> wrote:
[ trimming cc list ]
> Now, I have not the real environment.
> I use iperf generate fragment packets;
> and I always change NIC rx irq's affinity cpu, to make sure frag_mem_limit reach to thresh.
> my test machine, CPU num is 384.
Oh well, that explains it.
> > > + if (frag_mem_limit(nf) > nf->low_thresh) {
> > > inet_frag_schedule_worker(f);
> > > + update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
> > > + }
You need to reduce this to a lower value.
Your cpu count * batch_value needs to be less than
low_thresh to avoid problems.
Wtih 384 cpus its close to 12 mbyte...
Perhaps do this:
update_frag_mem_limit(nf, 2 * 1024*1024 / NR_CPUS);
However, I think its better to revert the percpu counter change and
move back to a single atomic_t count.
^ permalink raw reply [flat|nested] 15+ messages in thread
* RE: Question about ip_defrag
2017-08-29 13:46 ` Florian Westphal
@ 2017-08-30 1:52 ` liujian (CE)
0 siblings, 0 replies; 15+ messages in thread
From: liujian (CE) @ 2017-08-30 1:52 UTC (permalink / raw)
To: Florian Westphal
Cc: Jesper Dangaard Brouer, netdev, Wangkefeng (Kevin), weiyongjun (A)
Best Regards,
liujian
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Florian Westphal
> Sent: Tuesday, August 29, 2017 9:47 PM
> To: liujian (CE)
> Cc: Florian Westphal; Jesper Dangaard Brouer; netdev@vger.kernel.org;
> Wangkefeng (Kevin); weiyongjun (A)
> Subject: Re: Question about ip_defrag
>
> liujian (CE) <liujian56@huawei.com> wrote:
>
> [ trimming cc list ]
>
> > Now, I have not the real environment.
> > I use iperf generate fragment packets; and I always change NIC rx
> > irq's affinity cpu, to make sure frag_mem_limit reach to thresh.
> > my test machine, CPU num is 384.
>
> Oh well, that explains it.
>
> > > > + if (frag_mem_limit(nf) > nf->low_thresh) {
> > > > inet_frag_schedule_worker(f);
> > > > + update_frag_mem_limit(nf, SKB_TRUESIZE(1500) * 16);
> > > > + }
>
> You need to reduce this to a lower value.
> Your cpu count * batch_value needs to be less than low_thresh to avoid
> problems.
>
> Wtih 384 cpus its close to 12 mbyte...
>
> Perhaps do this:
>
> update_frag_mem_limit(nf, 2 * 1024*1024 / NR_CPUS);
>
>
> However, I think its better to revert the percpu counter change and move back
> to a single atomic_t count.
Ok.
Florian and Jesper, many thanks for this issue.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-29 7:53 ` Florian Westphal
@ 2017-08-30 10:58 ` Jesper Dangaard Brouer
2017-08-30 11:58 ` Florian Westphal
0 siblings, 1 reply; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2017-08-30 10:58 UTC (permalink / raw)
To: Florian Westphal
Cc: liujian (CE), davem, edumazet, netdev, Wangkefeng (Kevin),
weiyongjun (A),
brouer
(trimmed CC list a bit)
On Tue, 29 Aug 2017 09:53:15 +0200 Florian Westphal <fw@strlen.de> wrote:
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> > On Mon, 28 Aug 2017 16:00:32 +0200
> > Florian Westphal <fw@strlen.de> wrote:
> >
> > > liujian (CE) <liujian56@huawei.com> wrote:
> > > > Hi
> > > >
> > > > I checked our 3.10 kernel, we had backported all percpu_counter
> > > > bug fix in lib/percpu_counter.c and include/linux/percpu_counter.h.
> > > > And I check 4.13-rc6, also has the issue if NIC's rx cpu num big enough.
> > > >
> > > > > > > > the issue:
> > > > > > > > Ip_defrag fail caused by frag_mem_limit reached 4M(frags.high_thresh).
> > > > > > > > At this moment,sum_frag_mem_limit is about 10K.
> > > >
> > > > So should we change ipfrag high/low thresh to a reasonable value ?
> > > > And if it is, is there a standard to change the value?
> > >
> > > Each cpu can have frag_percpu_counter_batch bytes rest doesn't know
> > > about so with 64 cpus that is ~8 mbyte.
> > >
> > > possible solutions:
> > > 1. reduce frag_percpu_counter_batch to 16k or so
> > > 2. make both low and high thresh depend on NR_CPUS
>
> I take 2) back. Its wrong to do this, for large NR_CPU values it
> would even overflow.
Alternatively solution 3:
Why do we want to maintain a (4MBytes) memory limit, across all CPUs?
Couldn't we just allow each CPU to have a memory limit?
> > To me it looks like we/I have been using the wrong API for comparing
> > against percpu_counters. I guess we should have used __percpu_counter_compare().
>
> Are you sure? For liujian use case (64 cores) it looks like we would
> always fall through to percpu_counter_sum() so we eat spinlock_irqsave
> cost for all compares.
>
> Before we entertain this we should consider reducing frag_percpu_counter_batch
> to a smaller value.
Yes, I agree, we really need to lower/reduce the frag_percpu_counter_batch.
As you say, else the __percpu_counter_compare() call will be useless
(around systems with >= 32 CPUs).
I think the bug is in frag_mem_limit(). It just reads the global
counter (fbc->count), without considering other CPUs can have upto 130K
that haven't been subtracted yet (due to 3M low limit, become dangerous
at >=24 CPUs). The __percpu_counter_compare() does the right thing,
and takes into account the number of (online) CPUs and batch size, to
account for this.
If we choose 16K (16384), and use __percpu_counter_compare(), then we
can scale to systems with 256 CPUs (4*1024*1024/16384=256), before this
memory accounting becomes more expensive (than not using percpu_counters).
But Liujian, reports he have a 384 CPU system, so he would still need
to increase the lower+high threshold.
$ grep -H . /proc/sys/net/ipv*/ip*frag_*_thresh /proc/sys/net/netfilter/nf_conntrack_frag6_*_thresh
/proc/sys/net/ipv4/ipfrag_high_thresh:4194304
/proc/sys/net/ipv4/ipfrag_low_thresh:3145728
/proc/sys/net/ipv6/ip6frag_high_thresh:4194304
/proc/sys/net/ipv6/ip6frag_low_thresh:3145728
/proc/sys/net/netfilter/nf_conntrack_frag6_high_thresh:4194304
/proc/sys/net/netfilter/nf_conntrack_frag6_low_thresh:3145728
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-30 10:58 ` Jesper Dangaard Brouer
@ 2017-08-30 11:58 ` Florian Westphal
2017-08-30 12:22 ` Jesper Dangaard Brouer
0 siblings, 1 reply; 15+ messages in thread
From: Florian Westphal @ 2017-08-30 11:58 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Florian Westphal, liujian (CE),
davem, edumazet, netdev, Wangkefeng (Kevin), weiyongjun (A)
Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> > I take 2) back. Its wrong to do this, for large NR_CPU values it
> > would even overflow.
>
> Alternatively solution 3:
> Why do we want to maintain a (4MBytes) memory limit, across all CPUs?
> Couldn't we just allow each CPU to have a memory limit?
Consider ipv4, ipv6, nf ipv6 defrag, 6lowpan, and 8k cpus... This will
render any limit useless.
> > > To me it looks like we/I have been using the wrong API for comparing
> > > against percpu_counters. I guess we should have used __percpu_counter_compare().
> >
> > Are you sure? For liujian use case (64 cores) it looks like we would
> > always fall through to percpu_counter_sum() so we eat spinlock_irqsave
> > cost for all compares.
> >
> > Before we entertain this we should consider reducing frag_percpu_counter_batch
> > to a smaller value.
>
> Yes, I agree, we really need to lower/reduce the frag_percpu_counter_batch.
> As you say, else the __percpu_counter_compare() call will be useless
> (around systems with >= 32 CPUs).
>
> I think the bug is in frag_mem_limit(). It just reads the global
> counter (fbc->count), without considering other CPUs can have upto 130K
> that haven't been subtracted yet (due to 3M low limit, become dangerous
> at >=24 CPUs). The __percpu_counter_compare() does the right thing,
> and takes into account the number of (online) CPUs and batch size, to
> account for this.
Right, I think we should at very least use __percpu_counter_compare
before denying a new frag queue allocation request.
I'll create a patch.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Question about ip_defrag
2017-08-30 11:58 ` Florian Westphal
@ 2017-08-30 12:22 ` Jesper Dangaard Brouer
0 siblings, 0 replies; 15+ messages in thread
From: Jesper Dangaard Brouer @ 2017-08-30 12:22 UTC (permalink / raw)
To: Florian Westphal
Cc: liujian (CE), davem, edumazet, netdev, Wangkefeng (Kevin),
weiyongjun (A),
brouer
On Wed, 30 Aug 2017 13:58:20 +0200
Florian Westphal <fw@strlen.de> wrote:
> Jesper Dangaard Brouer <brouer@redhat.com> wrote:
> > > I take 2) back. Its wrong to do this, for large NR_CPU values it
> > > would even overflow.
> >
> > Alternatively solution 3:
> > Why do we want to maintain a (4MBytes) memory limit, across all CPUs?
> > Couldn't we just allow each CPU to have a memory limit?
>
> Consider ipv4, ipv6, nf ipv6 defrag, 6lowpan, and 8k cpus... This will
> render any limit useless.
With 8K CPUs I agree, that this might be a bad idea!
> > > > To me it looks like we/I have been using the wrong API for comparing
> > > > against percpu_counters. I guess we should have used __percpu_counter_compare().
> > >
> > > Are you sure? For liujian use case (64 cores) it looks like we would
> > > always fall through to percpu_counter_sum() so we eat spinlock_irqsave
> > > cost for all compares.
> > >
> > > Before we entertain this we should consider reducing frag_percpu_counter_batch
> > > to a smaller value.
> >
> > Yes, I agree, we really need to lower/reduce the frag_percpu_counter_batch.
> > As you say, else the __percpu_counter_compare() call will be useless
> > (around systems with >= 32 CPUs).
> >
> > I think the bug is in frag_mem_limit(). It just reads the global
> > counter (fbc->count), without considering other CPUs can have upto 130K
> > that haven't been subtracted yet (due to 3M low limit, become dangerous
> > at >=24 CPUs). The __percpu_counter_compare() does the right thing,
> > and takes into account the number of (online) CPUs and batch size, to
> > account for this.
>
> Right, I think we should at very least use __percpu_counter_compare
> before denying a new frag queue allocation request.
>
> I'll create a patch.
Oh, I've already started working on a patch, that I'm testing now. But
if you want to take the assignment then I'm fine with that!. I just
though that it was my responsibility to fix, given I introduced
percpu_counter usage (back in 2013-01-28 / 6d7b857d541e).
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2017-08-30 12:22 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <4F88C5DDA1E80143B232E89585ACE27D018F07E2@DGGEMA502-MBX.china.huawei.com>
2017-08-24 13:53 ` Question about ip_defrag Jesper Dangaard Brouer
[not found] ` <4F88C5DDA1E80143B232E89585ACE27D018F0AE1@DGGEMA502-MBX.china.huawei.com>
2017-08-24 18:59 ` Jesper Dangaard Brouer
2017-08-25 1:33 ` liujian (CE)
2017-08-28 8:08 ` liujian (CE)
2017-08-28 14:00 ` Florian Westphal
2017-08-29 7:20 ` Jesper Dangaard Brouer
2017-08-29 7:44 ` liujian (CE)
2017-08-29 7:53 ` Florian Westphal
2017-08-30 10:58 ` Jesper Dangaard Brouer
2017-08-30 11:58 ` Florian Westphal
2017-08-30 12:22 ` Jesper Dangaard Brouer
2017-08-29 7:40 ` liujian (CE)
2017-08-29 13:01 ` liujian (CE)
2017-08-29 13:46 ` Florian Westphal
2017-08-30 1:52 ` liujian (CE)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).