[PATCH v2 net-next] tc: remove unused redirect ttl

[PATCH v2 net-next] tc: remove unused redirect ttl

[Alexei Starovoitov]

improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
The code is from Jamal. I only did testing and benchmarking.

V1->V2: missed removal of unused 'dev' variable.

 include/uapi/linux/pkt_cls.h |    2 ++
 net/core/dev.c               |    9 ---------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index bf08e76bf505..f6344a522cd6 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -62,11 +62,13 @@ bits 9,10,11: redirect counter -  redirect TTL. Loop avoidance
 #define SET_TC_NCLS(v)   ( TC_NCLS | (v & ~TC_NCLS))
 #define CLR_TC_NCLS(v)   ( v & ~TC_NCLS)
 
+#ifndef __KERNEL__
 #define S_TC_RTTL          _TC_MAKE32(9)
 #define M_TC_RTTL          _TC_MAKEMASK(3,S_TC_RTTL)
 #define G_TC_RTTL(x)       _TC_GETVALUE(x,S_TC_RTTL,M_TC_RTTL)
 #define V_TC_RTTL(x)       _TC_MAKEVALUE(x,S_TC_RTTL)
 #define SET_TC_RTTL(v,n)   ((V_TC_RTTL(n)) | (v & ~M_TC_RTTL))
+#endif
 
 #define S_TC_AT          _TC_MAKE32(12)
 #define M_TC_AT          _TC_MAKEMASK(2,S_TC_AT)
diff --git a/net/core/dev.c b/net/core/dev.c
index c7ba0388f1be..97a15ae8d07a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3531,18 +3531,9 @@ EXPORT_SYMBOL_GPL(br_fdb_test_addr_hook);
  */
 static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
 {
-	struct net_device *dev = skb->dev;
-	u32 ttl = G_TC_RTTL(skb->tc_verd);
 	int result = TC_ACT_OK;
 	struct Qdisc *q;
 
-	if (unlikely(MAX_RED_LOOP < ttl++)) {
-		net_warn_ratelimited("Redir loop detected Dropping packet (%d->%d)\n",
-				     skb->skb_iif, dev->ifindex);
-		return TC_ACT_SHOT;
-	}
-
-	skb->tc_verd = SET_TC_RTTL(skb->tc_verd, ttl);
 	skb->tc_verd = SET_TC_AT(skb->tc_verd, AT_INGRESS);
 
 	q = rcu_dereference(rxq->qdisc);
-- 
1.7.9.5

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Florian Westphal]

Alexei Starovoitov <ast@plumgrid.com> wrote:
> improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps
> 
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> The code is from Jamal. I only did testing and benchmarking.

I had similar patch, so
Acked-by: Florian Westphal <fw@strlen.de>

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Sergei Shtylyov]

Hello.

On 5/1/2015 6:11 AM, Alexei Starovoitov wrote:

> improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps

> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
> The code is from Jamal. I only did testing and benchmarking.

    Then your patch should have had:

From: Jamal Hadi Salim <jhs@mojatatu.com>

before the change log, no?

> V1->V2: missed removal of unused 'dev' variable.

[...]

WBR, Sergei

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Daniel Borkmann]

On 05/01/2015 05:11 AM, Alexei Starovoitov wrote:
> improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps
>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Alexei Starovoitov]

On 5/1/15 3:18 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 5/1/2015 6:11 AM, Alexei Starovoitov wrote:
>
>> improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps
>
>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>> ---
>> The code is from Jamal. I only did testing and benchmarking.
>
>     Then your patch should have had:
>
> From: Jamal Hadi Salim <jhs@mojatatu.com>
>
> before the change log, no?

I don't think that's how it works. I've applied Jamal's patch
then fixed few minor things. Therefore his SOB is on top and
mine below. 'From' has no DCO meaning. It's used from time to time
in merge request summary, but that's a different case.

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Sergei Shtylyov]

On 5/1/2015 8:28 PM, Alexei Starovoitov wrote:

>>> improves ingress+u32 performance from 22.4 Mpps to 22.9 Mpps

>>> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
>>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>>> ---
>>> The code is from Jamal. I only did testing and benchmarking.

>>     Then your patch should have had:

>> From: Jamal Hadi Salim <jhs@mojatatu.com>

>> before the change log, no?

> I don't think that's how it works. I've applied Jamal's patch
> then fixed few minor things.

    You just said "I only did testing and benchmarking." :-)

> Therefore his SOB is on top and
> mine below. 'From' has no DCO meaning.

    Sorry, I don't know what DCO is, at least not in this context. :-)

> It's used from time to time
> in merge request summary, but that's a different case.

    I'm sure it's also used when you're submitting someone other's patches.

WBR, Sergei

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Cong Wang]

On Fri, May 1, 2015 at 10:28 AM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> I don't think that's how it works. I've applied Jamal's patch
> then fixed few minor things. Therefore his SOB is on top and
> mine below. 'From' has no DCO meaning. It's used from time to time
> in merge request summary, but that's a different case.


Quote from Documentation/SubmittingPatches:

"
The "from" line must be the very first line in the message body,
and has the form:

        From: Original Author <author@example.com>

The "from" line specifies who will be credited as the author of the
patch in the permanent changelog.  If the "from" line is missing,
then the "From:" line from the email header will be used to determine
the patch author in the changelog.
"

You should take a look at how Andrew Morton works
when he fixes minor things, I am sure he fixes much
more than you do.

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[David Miller]

From: Cong Wang <cwang@twopensource.com>
Date: Fri, 1 May 2015 13:33:28 -0700

> Quote from Documentation/SubmittingPatches:
> 
> "
> The "from" line must be the very first line in the message body,
> and has the form:
> 
>         From: Original Author <author@example.com>
> 
> The "from" line specifies who will be credited as the author of the
> patch in the permanent changelog.  If the "from" line is missing,
> then the "From:" line from the email header will be used to determine
> the patch author in the changelog.
> "
> 
> You should take a look at how Andrew Morton works
> when he fixes minor things, I am sure he fixes much
> more than you do.

+1

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Alexei Starovoitov]

On 5/1/15 4:48 PM, David Miller wrote:
> From: Cong Wang <cwang@twopensource.com>
> Date: Fri, 1 May 2015 13:33:28 -0700
>
>> Quote from Documentation/SubmittingPatches:
>>
>> "
>> The "from" line must be the very first line in the message body,
>> and has the form:
>>
>>          From: Original Author <author@example.com>
>>
>> The "from" line specifies who will be credited as the author of the
>> patch in the permanent changelog.  If the "from" line is missing,
>> then the "From:" line from the email header will be used to determine
>> the patch author in the changelog.
>> "
>>
>> You should take a look at how Andrew Morton works
>> when he fixes minor things, I am sure he fixes much
>> more than you do.
>
> +1
>

Dave, please help me understand the rules here.
Jamal gave me untested patch that had 8 lines deleted and said
"untested... to push it please go ahead."
I've removed one more line and added 2 extra.
My understanding is that 'From' applies to 100% authorship, no?
If no, where is the line?
I've seen people adding 'joint work' line and add sobs in order
of contribution % where the guy who sends it to the list is the
last sob, right?
Or any follow up work that based even on 10% of initial patch should
have 'from' of the first author? That doesn't sound right.

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[David Miller]

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Fri, 01 May 2015 20:15:49 -0700

> On 5/1/15 4:48 PM, David Miller wrote:
>> From: Cong Wang <cwang@twopensource.com>
>> Date: Fri, 1 May 2015 13:33:28 -0700
>>
>>> Quote from Documentation/SubmittingPatches:
>>>
>>> "
>>> The "from" line must be the very first line in the message body,
>>> and has the form:
>>>
>>>          From: Original Author <author@example.com>
>>>
>>> The "from" line specifies who will be credited as the author of the
>>> patch in the permanent changelog.  If the "from" line is missing,
>>> then the "From:" line from the email header will be used to determine
>>> the patch author in the changelog.
>>> "
>>>
>>> You should take a look at how Andrew Morton works
>>> when he fixes minor things, I am sure he fixes much
>>> more than you do.
>>
>> +1
>>
> 
> Dave, please help me understand the rules here.

If Jamal wrote the patch and you made some modifications it's
still his his patch so he gets authorship.

Who are you, Napoleon?

Re: [PATCH v2 net-next] tc: remove unused redirect ttl

[Alexei Starovoitov]

On 5/1/15 9:23 PM, David Miller wrote:
>
> If Jamal wrote the patch and you made some modifications it's
> still his his patch so he gets authorship.

ok. no problem.

> Who are you, Napoleon?

is it a joke? If it is, it's a bad one.
I simply asked what are the rules for 'from' tag.
I don't care a single bit who gets the authorship.

Everyone on this list is looking at you as a model of behavior.
Let's keep it technical.