public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: jamal <hadi@cyberus.ca>
To: Jiri Pirko <jpirko@redhat.com>
Cc: netdev@vger.kernel.org, davem@davemloft.net,
	herbert@gondor.hengli.com.au, kaber@trash.net
Subject: Re: Question about an assignment in handle_ing()
Date: Tue, 25 May 2010 05:51:07 -0400	[thread overview]
Message-ID: <1274781067.3878.872.camel@bigi> (raw)
In-Reply-To: <20100524112236.GF2810@psychotron.lab.eng.brq.redhat.com>

Hi Jiri,

On Mon, 2010-05-24 at 13:22 +0200, Jiri Pirko wrote:

> Question is if this code is correct here. Maybe I'm missing something but
> why is this dependent on a ptype was found previously?

The code is correct. 

Main reason for the else condn is driven by optimization:
If you are running tcpdump or other af packet type code, the "if" condn
is hit and matching actions are not allowed trample on that same packet
data. They have to make a private copy; otherwise, the "else" is hit and
(for optimization reason) you give ok to the actions that follow to
munge the packet. Essentially, you dont want actions to alloc/copy every
single time when you are not running tcpdump for example; reason is that
most of the time you run tcpdump it is for debugging.
[I had seen very observable differences on some old mips board back in
the day on whether you avoided copy every time vs when debugging by
running tcpdump and copied every packet.]

There is a secondary reason for the else stmnt: you want tcpdump to see
the naked packet as it came on say eth0. I am in travel mode at the
moment, so i cant validate this for you via a testcase (which moves the
else outside), but i know this was a problem back then...
Example. If i had a packet sequence path as follows: 
--> eth0-->tcpdump0-->filter match --> action: edit -->action: redirect
to dummy0-->tcpdump1-->dummy0(drop and count)
Then you want the packet on tcpdump0 to be whatever was seen by eth0.
You want to see on tcpdump1 whatever was seen by dummy0 - which is an
edited version of whatever was seen by eth0. 

If this description makes sense to you (and since this has come up more
than once before), would you be kind enough to submit a patch that fixes
the current comment and add my ACK to it?

cheers,
jamal


  reply	other threads:[~2010-05-25  9:51 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-24 11:22 Question about an assignment in handle_ing() Jiri Pirko
2010-05-25  9:51 ` jamal [this message]
2010-05-25 10:26   ` Herbert Xu
2010-05-25 12:03     ` jamal
2010-05-25 12:12       ` Herbert Xu
2010-05-25 12:20         ` jamal
2010-05-25 12:46           ` Herbert Xu
2010-05-25 13:13             ` jamal
2010-05-25 23:13               ` Herbert Xu
2010-05-26 11:38                 ` jamal
2010-05-30 13:29                   ` jamal
2010-06-03  8:01                     ` Herbert Xu
2010-06-03 12:43                       ` jamal
2010-06-03 12:47                         ` Herbert Xu
2010-06-03 12:53                           ` jamal
2010-06-03 12:56                             ` Herbert Xu
2010-06-03 12:58                             ` Herbert Xu
2010-06-03 12:58                             ` jamal
2010-06-03 13:00                               ` Herbert Xu
2010-06-03 13:01                                 ` jamal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1274781067.3878.872.camel@bigi \
    --to=hadi@cyberus.ca \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.hengli.com.au \
    --cc=jpirko@redhat.com \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox