* RE: [PATCH V2] tipc: Use bsearch library function
From: Jon Maloy @ 2017-09-16 13:20 UTC (permalink / raw)
To: Joe Perches, Ying Xue, Thomas Meyer, netdev@vger.kernel.org,
tipc-discussion@lists.sourceforge.net,
linux-kernel@vger.kernel.org, davem@davemloft.net
In-Reply-To: <1505557061.16316.7.camel@perches.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Joe Perches
> Sent: Saturday, September 16, 2017 06:18
> To: Ying Xue <ying.xue@windriver.com>; Thomas Meyer
> <thomas@m3y3r.de>; Jon Maloy <jon.maloy@ericsson.com>;
> netdev@vger.kernel.org; tipc-discussion@lists.sourceforge.net; linux-
> kernel@vger.kernel.org; davem@davemloft.net
> Subject: Re: [PATCH V2] tipc: Use bsearch library function
>
> On Sat, 2017-09-16 at 18:10 +0800, Ying Xue wrote:
> > On 09/16/2017 05:58 PM, Joe Perches wrote:
> > > On Sat, 2017-09-16 at 17:36 +0800, Ying Xue wrote:
> > > > On 09/16/2017 05:26 PM, Joe Perches wrote:
> > > > > On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
> > > > > > On 09/16/2017 03:50 PM, Thomas Meyer wrote:
> > > > > > > Use common library function rather than explicitly coding
> > > > > > > some variant of it yourself.
> > > > > > >
> > > > > > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> > > > > >
> > > > > > Acked-by: Ying Xue <ying.xue@windriver.com>
> > > > >
> > > > > Are you sure you want to do this?
> > > > >
> > > > > Note the comment above nameseq_find_subseq
> > > > >
> > > > > * Very time-critical, so binary searches through sub-sequence array.
> > > > >
> > > > > What impact does this change have on performance?
> > > >
> > > > Sorry, I couldn't see any essential difference between this new
> > > > implementation and the original one except that the former tries
> > > > to use the library function - bsearch() to replace the original
> > > > binary search algorithm implemented in TIPC itself. Therefore, I
> > > > don't think the change will have a big impact on performance.
> > > >
> > > > If I miss something, please let me know.
> > >
> > > Comparison via a function pointer in bsearch is slower than direct
> > > code without the function call overhead.
> > >
> >
> > Right, but probably we can tolerate the slight sacrifice here.
>
> What part of "very time critical" have you verified and benchmarked as
> inconsequential?
>
> Please post your results.
I agree with Joe here. This change does not simplify anything, it does not reduce the amount of code, plus that it introduce an unnecessary outline call in a place where we have every reason to let the compiler do its optimization job properly.
///jon
^ permalink raw reply
* Re: Use after free in __dst_destroy_metrics_generic
From: Julian Anastasov @ 2017-09-16 12:40 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan
Cc: Eric Dumazet, Cong Wang, Linux Kernel Network Developers,
Lorenzo Colitti
In-Reply-To: <0b4e9dbc852c4df7401dc770f357ae6a@codeaurora.org>
Hello,
On Fri, 15 Sep 2017, Subash Abhinov Kasiviswanathan wrote:
> > May be I'm missing some posting but I don't see if
> > the patch was tested successfully.
> >
> Hi Julian
>
> I've had this patch being tested for the last 3-4 days in our regression rack
> and I haven't seen the same issue being reproduced or even a related crash
> or leak in dst.
For now I see only its bug-hiding side effects, i.e.
it does not call kfree. Now if there is no double-free
there should be a leak, not for dst but for metrics.
> The original issue was reported only once to us from the regression rack only
> so the exact steps to reproduce is unknown.
OK, lets see, may be others can explain what happens.
Regards
^ permalink raw reply
* [PATCH net] net/sched: cls_matchall: fix crash when used with classful qdisc
From: Davide Caratti @ 2017-09-16 12:02 UTC (permalink / raw)
To: Jiri Pirko, Jamal Hadi Salim, Jiri Benc, David S . Miller; +Cc: netdev
this script, edited from Linux Advanced Routing and Traffic Control guide
tc q a dev en0 root handle 1: htb default a
tc c a dev en0 parent 1: classid 1:1 htb rate 6mbit burst 15k
tc c a dev en0 parent 1:1 classid 1:a htb rate 5mbit ceil 6mbit burst 15k
tc c a dev en0 parent 1:1 classid 1:b htb rate 1mbit ceil 6mbit burst 15k
tc f a dev en0 parent 1:0 prio 1 $clsname $clsargs classid 1:b
ping $address -c1
tc -s c s dev en0
classifies traffic to 1:b or 1:a, depending on whether the packet matches
or not the pattern $clsargs of filter $clsname. However, when $clsname is
'matchall', a systematic crash can be observed in htb_classify(). HTB and
classful qdiscs don't assign initial value to struct tcf_result, but then
they expect it to contain valid values after filters have been run. Thus,
current 'matchall' ignores the TCA_MATCHALL_CLASSID attribute, configured
by user, and makes HTB (and classful qdiscs) dereference random pointers.
By assigning head->res to *res in mall_classify(), before the actions are
invoked, we fix this crash and enable TCA_MATCHALL_CLASSID functionality,
that had no effect on 'matchall' classifier since its first introduction.
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1460213
Reported-by: Jiri Benc <jbenc@redhat.com>
Fixes: b87f7936a932 ("net/sched: introduce Match-all classifier")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/cls_matchall.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 21cc45caf842..eeac606c95ab 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -32,6 +32,7 @@ static int mall_classify(struct sk_buff *skb, const struct tcf_proto *tp,
if (tc_skip_sw(head->flags))
return -1;
+ *res = head->res;
return tcf_exts_exec(skb, &head->exts, res);
}
--
2.13.5
^ permalink raw reply related
* Re: [PATCH V2] tipc: Use bsearch library function
From: Joe Perches @ 2017-09-16 10:17 UTC (permalink / raw)
To: Ying Xue, Thomas Meyer, jon.maloy, netdev, tipc-discussion,
linux-kernel, davem
In-Reply-To: <35f41984-22e9-5adc-0e4d-a4ef4204f6d7@windriver.com>
On Sat, 2017-09-16 at 18:10 +0800, Ying Xue wrote:
> On 09/16/2017 05:58 PM, Joe Perches wrote:
> > On Sat, 2017-09-16 at 17:36 +0800, Ying Xue wrote:
> > > On 09/16/2017 05:26 PM, Joe Perches wrote:
> > > > On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
> > > > > On 09/16/2017 03:50 PM, Thomas Meyer wrote:
> > > > > > Use common library function rather than explicitly coding
> > > > > > some variant of it yourself.
> > > > > >
> > > > > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> > > > >
> > > > > Acked-by: Ying Xue <ying.xue@windriver.com>
> > > >
> > > > Are you sure you want to do this?
> > > >
> > > > Note the comment above nameseq_find_subseq
> > > >
> > > > * Very time-critical, so binary searches through sub-sequence array.
> > > >
> > > > What impact does this change have on performance?
> > >
> > > Sorry, I couldn't see any essential difference between this new
> > > implementation and the original one except that the former tries to use
> > > the library function - bsearch() to replace the original binary search
> > > algorithm implemented in TIPC itself. Therefore, I don't think the
> > > change will have a big impact on performance.
> > >
> > > If I miss something, please let me know.
> >
> > Comparison via a function pointer in bsearch is slower
> > than direct code without the function call overhead.
> >
>
> Right, but probably we can tolerate the slight sacrifice here.
What part of "very time critical" have you verified
and benchmarked as inconsequential?
Please post your results.
^ permalink raw reply
* Re: [PATCH V2] tipc: Use bsearch library function
From: Ying Xue @ 2017-09-16 10:10 UTC (permalink / raw)
To: Joe Perches, Thomas Meyer, jon.maloy, netdev, tipc-discussion,
linux-kernel, davem
In-Reply-To: <1505555908.16316.5.camel@perches.com>
On 09/16/2017 05:58 PM, Joe Perches wrote:
> On Sat, 2017-09-16 at 17:36 +0800, Ying Xue wrote:
>> On 09/16/2017 05:26 PM, Joe Perches wrote:
>>> On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
>>>> On 09/16/2017 03:50 PM, Thomas Meyer wrote:
>>>>> Use common library function rather than explicitly coding
>>>>> some variant of it yourself.
>>>>>
>>>>> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>>>>
>>>> Acked-by: Ying Xue <ying.xue@windriver.com>
>>>
>>> Are you sure you want to do this?
>>>
>>> Note the comment above nameseq_find_subseq
>>>
>>> * Very time-critical, so binary searches through sub-sequence array.
>>>
>>> What impact does this change have on performance?
>>
>> Sorry, I couldn't see any essential difference between this new
>> implementation and the original one except that the former tries to use
>> the library function - bsearch() to replace the original binary search
>> algorithm implemented in TIPC itself. Therefore, I don't think the
>> change will have a big impact on performance.
>>
>> If I miss something, please let me know.
>
> Comparison via a function pointer in bsearch is slower
> than direct code without the function call overhead.
>
Right, but probably we can tolerate the slight sacrifice here.
>
^ permalink raw reply
* Re: [PATCH V2] tipc: Use bsearch library function
From: Joe Perches @ 2017-09-16 9:58 UTC (permalink / raw)
To: Ying Xue, Thomas Meyer, jon.maloy, netdev, tipc-discussion,
linux-kernel, davem
In-Reply-To: <64ee51ce-eb7e-ac1c-56a9-9481f6f80b35@windriver.com>
On Sat, 2017-09-16 at 17:36 +0800, Ying Xue wrote:
> On 09/16/2017 05:26 PM, Joe Perches wrote:
> > On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
> > > On 09/16/2017 03:50 PM, Thomas Meyer wrote:
> > > > Use common library function rather than explicitly coding
> > > > some variant of it yourself.
> > > >
> > > > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
> > >
> > > Acked-by: Ying Xue <ying.xue@windriver.com>
> >
> > Are you sure you want to do this?
> >
> > Note the comment above nameseq_find_subseq
> >
> > * Very time-critical, so binary searches through sub-sequence array.
> >
> > What impact does this change have on performance?
>
> Sorry, I couldn't see any essential difference between this new
> implementation and the original one except that the former tries to use
> the library function - bsearch() to replace the original binary search
> algorithm implemented in TIPC itself. Therefore, I don't think the
> change will have a big impact on performance.
>
> If I miss something, please let me know.
Comparison via a function pointer in bsearch is slower
than direct code without the function call overhead.
^ permalink raw reply
* Re: [PATCH v2 0/2] enable hires timer to timeout datagram socket
From: Thomas Gleixner @ 2017-09-16 9:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eduardo Valentin, David Miller, dwmw2, vallish, shuah,
richardcochran, xiyou.wangcong, netdev, linux-kernel, anchalag,
dwmw
In-Reply-To: <1504897909.15310.89.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, 8 Sep 2017, Eric Dumazet wrote:
> On Fri, 2017-09-08 at 11:55 -0700, Eduardo Valentin wrote:
> > Hello,
> >
> > On Fri, Sep 08, 2017 at 10:26:45AM -0700, David Miller wrote:
> > > From: David Woodhouse <dwmw2@infradead.org>
> > > Date: Fri, 08 Sep 2017 18:23:22 +0100
> > >
> > > > I don't know that anyone's ever tried saying "show me the chapter
> > and
> > > > verse of the documentation"
> > >
> > > Do you know why I brought this up? Because the person I am replying
> > > to told me that the syscall documentation should have suggested this
> > > or that.
> > >
> > > That's why.
> >
> > :-) My intention was for sure not to upset anybody.
> >
> > Just to reiterate, the point of patch is simple, there was a change in
> > behavior in the system call from one kernel version to the other. As I
> > mentioned, I agree that the userspace could use other means to achieve
> > the same, but still the system call behavior has changed.
> >
> > >
> > > So let's concentrate on the other aspects of my reply, ok?
> >
> > I agree. I would prefer to understand here what is the technical
> > reason not to accept these patches other than "use other system
> > calls".
>
> So if we need to replace all 'legacy' timers to high resolution timer,
> because some application was _relying_ on jiffies being kind of precise,
> maybe it is better to revert the change done on legacy timers.
Which would be a major step back in terms of timer performance and system
disturbance caused by massive recascading operations.
> Or continue the migration and make them use high res internally.
>
> select() and poll() are the standard way to have precise timeouts,
> it is silly we have to maintain a timeout handling in the datagram fast
> path.
A few years ago we switched select/poll over to use hrtimers because the
wheel timers were too inaccurate for some operations, so it feels
consequent to switch the timeout in the datagram rcv path over as well. I
agree that the whole timeout magic there feels silly, but unfortunately
it's a documented property of sockets.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH V2] tipc: Use bsearch library function
From: Ying Xue @ 2017-09-16 9:36 UTC (permalink / raw)
To: Joe Perches, Thomas Meyer, jon.maloy, netdev, tipc-discussion,
linux-kernel, davem
In-Reply-To: <1505553970.16316.1.camel@perches.com>
On 09/16/2017 05:26 PM, Joe Perches wrote:
> On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
>> On 09/16/2017 03:50 PM, Thomas Meyer wrote:
>>> Use common library function rather than explicitly coding
>>> some variant of it yourself.
>>>
>>> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>>
>> Acked-by: Ying Xue <ying.xue@windriver.com>
>
> Are you sure you want to do this?
>
> Note the comment above nameseq_find_subseq
>
> * Very time-critical, so binary searches through sub-sequence array.
>
> What impact does this change have on performance?
Sorry, I couldn't see any essential difference between this new
implementation and the original one except that the former tries to use
the library function - bsearch() to replace the original binary search
algorithm implemented in TIPC itself. Therefore, I don't think the
change will have a big impact on performance.
If I miss something, please let me know.
Thanks,
Ying
>
>>> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
>>> index bd0aac87b41a..eeb4d7a13de2 100644
>>> --- a/net/tipc/name_table.c
>>> +++ b/net/tipc/name_table.c
>>> @@ -44,6 +44,7 @@
>>> #include "addr.h"
>>> #include "node.h"
>>> #include <net/genetlink.h>
>>> +#include <linux/bsearch.h>
>>>
>>> #define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
>>>
>>> @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
>>> return nseq;
>>> }
>>>
>>> +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
>>> +{
>>> + struct sub_seq *sseq = (struct sub_seq *)elt;
>>> + u32 instance = *(u32 *)key;
>>> +
>>> + if (instance < sseq->lower)
>>> + return -1;
>>> + else if (instance > sseq->upper)
>>> + return 1;
>>> + return 0;
>>> +}
>>> +
>>> /**
>>> * nameseq_find_subseq - find sub-sequence (if any) matching a name instance
>>> *
>>> @@ -176,21 +189,8 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
>>> static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
>>> u32 instance)
>>> {
>>> - struct sub_seq *sseqs = nseq->sseqs;
>>> - int low = 0;
>>> - int high = nseq->first_free - 1;
>>> - int mid;
>>> -
>>> - while (low <= high) {
>>> - mid = (low + high) / 2;
>>> - if (instance < sseqs[mid].lower)
>>> - high = mid - 1;
>>> - else if (instance > sseqs[mid].upper)
>>> - low = mid + 1;
>>> - else
>>> - return &sseqs[mid];
>>> - }
>>> - return NULL;
>>> + return bsearch(&instance, nseq->sseqs, nseq->first_free,
>>> + sizeof(struct sub_seq), nameseq_find_subseq_cmp);
>>> }
>>>
>>> /**
>>>
>
^ permalink raw reply
* Re: [PATCH V2] tipc: Use bsearch library function
From: Joe Perches @ 2017-09-16 9:26 UTC (permalink / raw)
To: Ying Xue, Thomas Meyer, jon.maloy, netdev, tipc-discussion,
linux-kernel, davem
In-Reply-To: <16128f5e-66ff-b6ec-c0e1-74ea08c212b0@windriver.com>
On Sat, 2017-09-16 at 17:02 +0800, Ying Xue wrote:
> On 09/16/2017 03:50 PM, Thomas Meyer wrote:
> > Use common library function rather than explicitly coding
> > some variant of it yourself.
> >
> > Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
>
> Acked-by: Ying Xue <ying.xue@windriver.com>
Are you sure you want to do this?
Note the comment above nameseq_find_subseq
* Very time-critical, so binary searches through sub-sequence array.
What impact does this change have on performance?
> > diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> > index bd0aac87b41a..eeb4d7a13de2 100644
> > --- a/net/tipc/name_table.c
> > +++ b/net/tipc/name_table.c
> > @@ -44,6 +44,7 @@
> > #include "addr.h"
> > #include "node.h"
> > #include <net/genetlink.h>
> > +#include <linux/bsearch.h>
> >
> > #define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
> >
> > @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> > return nseq;
> > }
> >
> > +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
> > +{
> > + struct sub_seq *sseq = (struct sub_seq *)elt;
> > + u32 instance = *(u32 *)key;
> > +
> > + if (instance < sseq->lower)
> > + return -1;
> > + else if (instance > sseq->upper)
> > + return 1;
> > + return 0;
> > +}
> > +
> > /**
> > * nameseq_find_subseq - find sub-sequence (if any) matching a name instance
> > *
> > @@ -176,21 +189,8 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> > static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
> > u32 instance)
> > {
> > - struct sub_seq *sseqs = nseq->sseqs;
> > - int low = 0;
> > - int high = nseq->first_free - 1;
> > - int mid;
> > -
> > - while (low <= high) {
> > - mid = (low + high) / 2;
> > - if (instance < sseqs[mid].lower)
> > - high = mid - 1;
> > - else if (instance > sseqs[mid].upper)
> > - low = mid + 1;
> > - else
> > - return &sseqs[mid];
> > - }
> > - return NULL;
> > + return bsearch(&instance, nseq->sseqs, nseq->first_free,
> > + sizeof(struct sub_seq), nameseq_find_subseq_cmp);
> > }
> >
> > /**
> >
^ permalink raw reply
* Re: [PATCH V2] tipc: Use bsearch library function
From: Ying Xue @ 2017-09-16 9:02 UTC (permalink / raw)
To: Thomas Meyer, jon.maloy, netdev, tipc-discussion, linux-kernel,
davem
In-Reply-To: <20170916075036.28676-1-thomas@m3y3r.de>
On 09/16/2017 03:50 PM, Thomas Meyer wrote:
> Use common library function rather than explicitly coding
> some variant of it yourself.
>
> Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
Acked-by: Ying Xue <ying.xue@windriver.com>
> ---
> net/tipc/name_table.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
>
> V2: Coding style
>
> diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
> index bd0aac87b41a..eeb4d7a13de2 100644
> --- a/net/tipc/name_table.c
> +++ b/net/tipc/name_table.c
> @@ -44,6 +44,7 @@
> #include "addr.h"
> #include "node.h"
> #include <net/genetlink.h>
> +#include <linux/bsearch.h>
>
> #define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
>
> @@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> return nseq;
> }
>
> +static int nameseq_find_subseq_cmp(const void *key, const void *elt)
> +{
> + struct sub_seq *sseq = (struct sub_seq *)elt;
> + u32 instance = *(u32 *)key;
> +
> + if (instance < sseq->lower)
> + return -1;
> + else if (instance > sseq->upper)
> + return 1;
> + return 0;
> +}
> +
> /**
> * nameseq_find_subseq - find sub-sequence (if any) matching a name instance
> *
> @@ -176,21 +189,8 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
> static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
> u32 instance)
> {
> - struct sub_seq *sseqs = nseq->sseqs;
> - int low = 0;
> - int high = nseq->first_free - 1;
> - int mid;
> -
> - while (low <= high) {
> - mid = (low + high) / 2;
> - if (instance < sseqs[mid].lower)
> - high = mid - 1;
> - else if (instance > sseqs[mid].upper)
> - low = mid + 1;
> - else
> - return &sseqs[mid];
> - }
> - return NULL;
> + return bsearch(&instance, nseq->sseqs, nseq->first_free,
> + sizeof(struct sub_seq), nameseq_find_subseq_cmp);
> }
>
> /**
>
^ permalink raw reply
* [PATCH V2] tipc: Use bsearch library function
From: Thomas Meyer @ 2017-09-16 7:50 UTC (permalink / raw)
To: jon.maloy, ying.xue, netdev, tipc-discussion, linux-kernel, davem
Cc: Thomas Meyer
In-Reply-To: <20170911.143025.555018840006192902.davem@davemloft.net>
Use common library function rather than explicitly coding
some variant of it yourself.
Signed-off-by: Thomas Meyer <thomas@m3y3r.de>
---
net/tipc/name_table.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
V2: Coding style
diff --git a/net/tipc/name_table.c b/net/tipc/name_table.c
index bd0aac87b41a..eeb4d7a13de2 100644
--- a/net/tipc/name_table.c
+++ b/net/tipc/name_table.c
@@ -44,6 +44,7 @@
#include "addr.h"
#include "node.h"
#include <net/genetlink.h>
+#include <linux/bsearch.h>
#define TIPC_NAMETBL_SIZE 1024 /* must be a power of 2 */
@@ -168,6 +169,18 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
return nseq;
}
+static int nameseq_find_subseq_cmp(const void *key, const void *elt)
+{
+ struct sub_seq *sseq = (struct sub_seq *)elt;
+ u32 instance = *(u32 *)key;
+
+ if (instance < sseq->lower)
+ return -1;
+ else if (instance > sseq->upper)
+ return 1;
+ return 0;
+}
+
/**
* nameseq_find_subseq - find sub-sequence (if any) matching a name instance
*
@@ -176,21 +189,8 @@ static struct name_seq *tipc_nameseq_create(u32 type, struct hlist_head *seq_hea
static struct sub_seq *nameseq_find_subseq(struct name_seq *nseq,
u32 instance)
{
- struct sub_seq *sseqs = nseq->sseqs;
- int low = 0;
- int high = nseq->first_free - 1;
- int mid;
-
- while (low <= high) {
- mid = (low + high) / 2;
- if (instance < sseqs[mid].lower)
- high = mid - 1;
- else if (instance > sseqs[mid].upper)
- low = mid + 1;
- else
- return &sseqs[mid];
- }
- return NULL;
+ return bsearch(&instance, nseq->sseqs, nseq->first_free,
+ sizeof(struct sub_seq), nameseq_find_subseq_cmp);
}
/**
--
2.11.0
^ permalink raw reply related
* Re: [Outreachy kernel] [PATCH] Staging: irda: Use !x instead of NULL comparison
From: Julia Lawall @ 2017-09-16 7:14 UTC (permalink / raw)
To: Srishti Sharma
Cc: samuel, gregkh, netdev, devel, linux-kernel, outreachy-kernel
In-Reply-To: <1505543667-4670-1-git-send-email-srishtishar@gmail.com>
On Sat, 16 Sep 2017, Srishti Sharma wrote:
> Test for NULL as !x where functions that return NULL on failure
> are used. Done using the following semantic patch by coccinelle.
>
> @ is_null @
> expression E;
> statement S;
> @@
>
> E = (\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap\|
> usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\)(...));
>
> (
> if(!E)
> S
> |
> -if(E==NULL)
> +if(!E)
> S
> )
>
> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
> drivers/staging/irda/net/discovery.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/irda/net/discovery.c b/drivers/staging/irda/net/discovery.c
> index 364d70a..1e54954 100644
> --- a/drivers/staging/irda/net/discovery.c
> +++ b/drivers/staging/irda/net/discovery.c
> @@ -179,7 +179,7 @@ void irlmp_expire_discoveries(hashbin_t *log, __u32 saddr, int force)
> /* Create the client specific buffer */
> n = HASHBIN_GET_SIZE(log);
> buffer = kmalloc(n * sizeof(struct irda_device_info), GFP_ATOMIC);
> - if (buffer == NULL) {
> + if (!buffer) {
> spin_unlock_irqrestore(&log->hb_spinlock, flags);
> return;
> }
> @@ -291,7 +291,7 @@ struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn,
> /* Create the client specific buffer */
> n = HASHBIN_GET_SIZE(log);
> buffer = kmalloc(n * sizeof(struct irda_device_info), GFP_ATOMIC);
> - if (buffer == NULL) {
> + if (!buffer) {
> spin_unlock_irqrestore(&log->hb_spinlock, flags);
> return NULL;
> }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505543667-4670-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* [PATCH] Staging: irda: Use !x instead of NULL comparison
From: Srishti Sharma @ 2017-09-16 6:34 UTC (permalink / raw)
To: samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
Srishti Sharma
Test for NULL as !x where functions that return NULL on failure
are used. Done using the following semantic patch by coccinelle.
@ is_null @
expression E;
statement S;
@@
E = (\(kmalloc\|devm_kzalloc\|kmalloc_array\|devm_ioremap\|
usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\)(...));
(
if(!E)
S
|
-if(E==NULL)
+if(!E)
S
)
Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
drivers/staging/irda/net/discovery.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/irda/net/discovery.c b/drivers/staging/irda/net/discovery.c
index 364d70a..1e54954 100644
--- a/drivers/staging/irda/net/discovery.c
+++ b/drivers/staging/irda/net/discovery.c
@@ -179,7 +179,7 @@ void irlmp_expire_discoveries(hashbin_t *log, __u32 saddr, int force)
/* Create the client specific buffer */
n = HASHBIN_GET_SIZE(log);
buffer = kmalloc(n * sizeof(struct irda_device_info), GFP_ATOMIC);
- if (buffer == NULL) {
+ if (!buffer) {
spin_unlock_irqrestore(&log->hb_spinlock, flags);
return;
}
@@ -291,7 +291,7 @@ struct irda_device_info *irlmp_copy_discoveries(hashbin_t *log, int *pn,
/* Create the client specific buffer */
n = HASHBIN_GET_SIZE(log);
buffer = kmalloc(n * sizeof(struct irda_device_info), GFP_ATOMIC);
- if (buffer == NULL) {
+ if (!buffer) {
spin_unlock_irqrestore(&log->hb_spinlock, flags);
return NULL;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: Florian Fainelli @ 2017-09-16 6:04 UTC (permalink / raw)
To: rosenp, Eric Dumazet; +Cc: netdev
In-Reply-To: <1505522322.3726.0.camel@gmail.com>
On September 15, 2017 5:38:42 PM PDT, rosenp@gmail.com wrote:
>I have not. Unfortunately I own no gigabit hardware to test this on.
>The MIPS CPU runs at 300MHz on my unit.
>
bgmac is used on Gigabit capable hardware, like Northstar and Northstar Plus, and others too, so unless you can get access to such HW or get confirmation from someone that your patches changes something, I would just drop this change and not bother. This is already not 100mbits/sec linerate...
>On Fri, 2017-09-15 at 17:34 -0700, Eric Dumazet wrote:
>> On Fri, 2017-09-15 at 17:23 -0700, Rosen Penev wrote:
>> > On a linksys E1200v1 (actually a crossflashed E1000v2), the
>> > offloading features give no measurable benefit to speed or latency.
>> > Furthermore, disabling GRO actually improves iperf performance by a
>> > whoppimg 3mbps. Results:
>> >
>> > Currently:
>> >
>> > v2: Changed napi_gro_receive to netif_receive_skb. Seems to have an
>> > identical result.
>> >
>> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
>> > ---
>> > drivers/net/ethernet/broadcom/bgmac.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c
>> > b/drivers/net/ethernet/broadcom/bgmac.c
>> > index 48d672b204a4..1fb0053aeee7 100644
>> > --- a/drivers/net/ethernet/broadcom/bgmac.c
>> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
>> > @@ -483,7 +483,7 @@ static int bgmac_dma_rx_read(struct bgmac
>> > *bgmac, struct bgmac_dma_ring *ring,
>> > skb->protocol = eth_type_trans(skb, bgmac-
>> > >net_dev);
>> > bgmac->net_dev->stats.rx_bytes += len;
>> > bgmac->net_dev->stats.rx_packets++;
>> > - napi_gro_receive(&bgmac->napi, skb);
>> > + netif_receive_skb(skb);
>> > handled++;
>> > } while (0);
>> >
>>
>> And have you tested 1Gbit link speed ?
>> ( Or 2.5 Gbit link speed )
>>
>> If you want to disable GRO on your host, fine : you can use ethtool
>> -K
>>
>>
>>
(please don't top-post)
--
Florian
^ permalink raw reply
* Re: [PATCH] once: switch to new jump label API
From: Eric Biggers @ 2017-09-16 4:07 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: David S. Miller, netdev, linux-kernel, Jason Baron,
Peter Zijlstra, Eric Biggers
In-Reply-To: <874lszmoh2.fsf@stressinduktion.org>
On Tue, Aug 22, 2017 at 02:44:41PM -0400, Hannes Frederic Sowa wrote:
> Eric Biggers <ebiggers3@gmail.com> writes:
>
> > From: Eric Biggers <ebiggers@google.com>
> >
> > Switch the DO_ONCE() macro from the deprecated jump label API to the new
> > one. The new one is more readable, and for DO_ONCE() it also makes the
> > generated code more icache-friendly: now the one-time initialization
> > code is placed out-of-line at the jump target, rather than at the inline
> > fallthrough case.
> >
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
>
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org.
>
> Thanks!
Great! Who though is the maintainer for this code? It seems it was originally
taken by David Miller through the networking tree. David, are you taking
further patches to the "once" functions, or should I be trying to get this into
-mm, or somewhere else?
Eric
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: David Miller @ 2017-09-16 3:56 UTC (permalink / raw)
To: eric.dumazet; +Cc: f.fainelli, rosenp, netdev, zajec5, nbd
In-Reply-To: <1505520210.29839.7.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 15 Sep 2017 17:03:30 -0700
> On Fri, 2017-09-15 at 15:54 -0700, Florian Fainelli wrote:
>> On September 15, 2017 3:22:18 PM PDT, Rosen Penev <rosenp@gmail.com>
>> wrote:
>> >On a linksys E1200v1 (actually a crossflashed E1000v2), the
>> offloading
>> >features give no measurable benefit to speed or latency. Furthermore,
>> >disabling GRO actually improves iperf performance by a whoppimg
>> 3mbps.
>>
>> Do you have a way to generate gigabit tests and see what results you
>> are getting? We probably are not going to see a 30% improvement just
>> by extrapolation.
>>
> +1
>
> It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the
> driver to handle these features.
>
> And of course GRO was not removed, meaning the bench results were non
> conclusive.
My sentiments exactly, I think this is a completely unwise change.
^ permalink raw reply
* Re: [PATCH net] tcp: fix data delivery rate
From: Soheil Hassas Yeganeh @ 2017-09-16 1:33 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, liujian, Eric Dumazet, Yuchung Cheng, Jerry Chu,
netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <1505519262.29839.5.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, Sep 15, 2017 at 7:47 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Now skb->mstamp_skb is updated later, we also need to call
> tcp_rate_skb_sent() after the update is done.
>
> Fixes: 8c72c65b426b ("tcp: update skb->skb_mstamp more carefully")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Nice catch!
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: rosenp @ 2017-09-16 0:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1505522096.29839.13.camel@edumazet-glaptop3.roam.corp.google.com>
I have not. Unfortunately I own no gigabit hardware to test this on.
The MIPS CPU runs at 300MHz on my unit.
On Fri, 2017-09-15 at 17:34 -0700, Eric Dumazet wrote:
> On Fri, 2017-09-15 at 17:23 -0700, Rosen Penev wrote:
> > On a linksys E1200v1 (actually a crossflashed E1000v2), the
> > offloading features give no measurable benefit to speed or latency.
> > Furthermore, disabling GRO actually improves iperf performance by a
> > whoppimg 3mbps. Results:
> >
> > Currently:
> >
> > v2: Changed napi_gro_receive to netif_receive_skb. Seems to have an
> > identical result.
> >
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > ---
> > drivers/net/ethernet/broadcom/bgmac.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/broadcom/bgmac.c
> > b/drivers/net/ethernet/broadcom/bgmac.c
> > index 48d672b204a4..1fb0053aeee7 100644
> > --- a/drivers/net/ethernet/broadcom/bgmac.c
> > +++ b/drivers/net/ethernet/broadcom/bgmac.c
> > @@ -483,7 +483,7 @@ static int bgmac_dma_rx_read(struct bgmac
> > *bgmac, struct bgmac_dma_ring *ring,
> > skb->protocol = eth_type_trans(skb, bgmac-
> > >net_dev);
> > bgmac->net_dev->stats.rx_bytes += len;
> > bgmac->net_dev->stats.rx_packets++;
> > - napi_gro_receive(&bgmac->napi, skb);
> > + netif_receive_skb(skb);
> > handled++;
> > } while (0);
> >
>
> And have you tested 1Gbit link speed ?
> ( Or 2.5 Gbit link speed )
>
> If you want to disable GRO on your host, fine : you can use ethtool
> -K
>
>
>
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: Eric Dumazet @ 2017-09-16 0:34 UTC (permalink / raw)
To: Rosen Penev; +Cc: netdev
In-Reply-To: <20170916002305.2794-1-rosenp@gmail.com>
On Fri, 2017-09-15 at 17:23 -0700, Rosen Penev wrote:
> On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:
>
> Currently:
>
> v2: Changed napi_gro_receive to netif_receive_skb. Seems to have an identical result.
>
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
> drivers/net/ethernet/broadcom/bgmac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
> index 48d672b204a4..1fb0053aeee7 100644
> --- a/drivers/net/ethernet/broadcom/bgmac.c
> +++ b/drivers/net/ethernet/broadcom/bgmac.c
> @@ -483,7 +483,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
> skb->protocol = eth_type_trans(skb, bgmac->net_dev);
> bgmac->net_dev->stats.rx_bytes += len;
> bgmac->net_dev->stats.rx_packets++;
> - napi_gro_receive(&bgmac->napi, skb);
> + netif_receive_skb(skb);
> handled++;
> } while (0);
>
And have you tested 1Gbit link speed ?
( Or 2.5 Gbit link speed )
If you want to disable GRO on your host, fine : you can use ethtool -K
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: Denys Fedoryshchenko @ 2017-09-16 0:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: rosenp, Florian Fainelli, netdev, zajec5, nbd, netdev-owner
In-Reply-To: <1505521095.29839.11.camel@edumazet-glaptop3.roam.corp.google.com>
On 2017-09-16 03:18, Eric Dumazet wrote:
> On Fri, 2017-09-15 at 17:10 -0700, rosenp@gmail.com wrote:
>> Ok fair enough. Will only disable GRO in the driver.
>
> Well, do not even try.
>
> NETIF_F_SOFT_FEATURES is set by core networking stack in
> register_netdevice(), ( commit 212b573f5552c60265da721ff9ce32e3462a2cdd
> )
>
> Absolutely no driver disables GRO (excepts the ones playing with LRO)
I believe also iperf is definitely inconclusive test.
Except iperf there is lot of different workloads and configurations,
that might have different results.
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: rosenp @ 2017-09-16 0:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Fainelli, netdev, zajec5, nbd
In-Reply-To: <1505521095.29839.11.camel@edumazet-glaptop3.roam.corp.google.com>
netif_receive_skb seems to have the same effect as "ethtool -K eth0 gro
off".
On Fri, 2017-09-15 at 17:18 -0700, Eric Dumazet wrote:
> On Fri, 2017-09-15 at 17:10 -0700, rosenp@gmail.com wrote:
> > Ok fair enough. Will only disable GRO in the driver.
>
> Well, do not even try.
>
> NETIF_F_SOFT_FEATURES is set by core networking stack in
> register_netdevice(), ( commit
> 212b573f5552c60265da721ff9ce32e3462a2cdd
> )
>
> Absolutely no driver disables GRO (excepts the ones playing with LRO)
>
>
>
>
>
>
^ permalink raw reply
* [PATCH] bgmac: Remove all offloading features, including GRO.
From: Rosen Penev @ 2017-09-16 0:23 UTC (permalink / raw)
To: netdev; +Cc: Rosen Penev
On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:
Currently:
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52410
[ ID] Interval Transfer Bandwidth Reads Dist(bin=16.0K)
[ 4] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 641 75:181:12:1:1:0:0:371
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52412
[ 5] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 629 51:194:13:1:0:1:0:369
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52414
[ 4] 0.00-10.02 sec 51.9 MBytes 43.4 Mbits/sec 695 126:203:1:0:0:0:2:363
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52416
[ 5] 0.00-10.01 sec 52.4 MBytes 43.9 Mbits/sec 626 57:186:10:0:0:0:0:373
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52420
[ 4] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 605 36:179:16:1:0:1:0:372
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52422
After disabling everything - including GRO:
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52440
[ ID] Interval Transfer Bandwidth Reads Dist(bin=16.0K)
[ 4] 0.00-10.01 sec 55.1 MBytes 46.2 Mbits/sec 672 180:82:0:0:1:0:0:409
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52442
[ 5] 0.00-10.01 sec 56.0 MBytes 46.9 Mbits/sec 636 117:96:0:0:1:0:0:422
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52444
[ 4] 0.00-10.01 sec 55.4 MBytes 46.4 Mbits/sec 675 172:92:0:0:1:0:0:410
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52446
[ 5] 0.00-10.01 sec 56.0 MBytes 46.9 Mbits/sec 633 119:90:0:1:1:0:0:422
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52448
[ 4] 0.00-10.01 sec 55.2 MBytes 46.3 Mbits/sec 688 157:123:0:0:2:0:0:406
v2: Changed napi_gro_receive to netif_receive_skb. Seems to have an identical result.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 48d672b204a4..1fb0053aeee7 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -483,7 +483,7 @@ static int bgmac_dma_rx_read(struct bgmac *bgmac, struct bgmac_dma_ring *ring,
skb->protocol = eth_type_trans(skb, bgmac->net_dev);
bgmac->net_dev->stats.rx_bytes += len;
bgmac->net_dev->stats.rx_packets++;
- napi_gro_receive(&bgmac->napi, skb);
+ netif_receive_skb(skb);
handled++;
} while (0);
--
2.13.5
^ permalink raw reply related
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: Eric Dumazet @ 2017-09-16 0:18 UTC (permalink / raw)
To: rosenp; +Cc: Florian Fainelli, netdev, zajec5, nbd
In-Reply-To: <1505520638.8507.2.camel@gmail.com>
On Fri, 2017-09-15 at 17:10 -0700, rosenp@gmail.com wrote:
> Ok fair enough. Will only disable GRO in the driver.
Well, do not even try.
NETIF_F_SOFT_FEATURES is set by core networking stack in
register_netdevice(), ( commit 212b573f5552c60265da721ff9ce32e3462a2cdd
)
Absolutely no driver disables GRO (excepts the ones playing with LRO)
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: rosenp @ 2017-09-16 0:10 UTC (permalink / raw)
To: Eric Dumazet, Florian Fainelli; +Cc: netdev, zajec5, nbd
In-Reply-To: <1505520210.29839.7.camel@edumazet-glaptop3.roam.corp.google.com>
Ok fair enough. Will only disable GRO in the driver.
Tests were done using "ethtool -K eth0 gro off" and on.
On Fri, 2017-09-15 at 17:03 -0700, Eric Dumazet wrote:
> On Fri, 2017-09-15 at 15:54 -0700, Florian Fainelli wrote:
> > On September 15, 2017 3:22:18 PM PDT, Rosen Penev <rosenp@gmail.com
> > >
> > wrote:
> > > On a linksys E1200v1 (actually a crossflashed E1000v2), the
> >
> > offloading
> > > features give no measurable benefit to speed or latency.
> > > Furthermore,
> > > disabling GRO actually improves iperf performance by a whoppimg
> >
> > 3mbps.
> >
> > Do you have a way to generate gigabit tests and see what results
> > you
> > are getting? We probably are not going to see a 30% improvement
> > just
> > by extrapolation.
> >
>
> +1
>
> It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM |
> NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the
> driver to handle these features.
>
> And of course GRO was not removed, meaning the bench results were non
> conclusive.
>
>
>
^ permalink raw reply
* Re: [PATCH] bgmac: Remove all offloading features, including GRO.
From: Eric Dumazet @ 2017-09-16 0:03 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Rosen Penev, netdev, zajec5, nbd
In-Reply-To: <252FFED5-906E-4D1B-9E87-679D0CE801D4@gmail.com>
On Fri, 2017-09-15 at 15:54 -0700, Florian Fainelli wrote:
> On September 15, 2017 3:22:18 PM PDT, Rosen Penev <rosenp@gmail.com>
> wrote:
> >On a linksys E1200v1 (actually a crossflashed E1000v2), the
> offloading
> >features give no measurable benefit to speed or latency. Furthermore,
> >disabling GRO actually improves iperf performance by a whoppimg
> 3mbps.
>
> Do you have a way to generate gigabit tests and see what results you
> are getting? We probably are not going to see a 30% improvement just
> by extrapolation.
>
+1
It seems silly to remove NETIF_F_SG | NETIF_F_IP_CSUM |
NETIF_F_IPV6_CSUM from dev->features, yet leave the dead-code in the
driver to handle these features.
And of course GRO was not removed, meaning the bench results were non
conclusive.
^ 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