All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jesper Dangaard Brouer <jbrouer@redhat.com>
To: Li Zhong <zhong@linux.vnet.ibm.com>
Cc: Daniel Borkmann <dborkman@redhat.com>,
	linux-next list <linux-next@vger.kernel.org>,
	brouer@redhat.com, davem@davemloft.net
Subject: Re: [RFC PATCH net-next] packet: fix using smp_processor_id() in preemptible code
Date: Thu, 12 Dec 2013 08:18:38 +0100	[thread overview]
Message-ID: <20131212081838.2d2eafb9@redhat.com> (raw)
In-Reply-To: <1386892503.2586.13.camel@ThinkPad-T5421>


On Fri, 13 Dec 2013 07:55:03 +0800 Li Zhong <zhong@linux.vnet.ibm.com> wrote:
> On Wed, 2013-12-11 at 10:55 +0100, Daniel Borkmann wrote:
> > On 12/12/2013 07:10 AM, Li Zhong wrote:
[...]
> > > Also, it seems that we could move skb_set_queue_mapping() into
> > > packet_pick_tx_queue(), so we avoid calling it one more time
> > > unnecessarily if we are going into the normal dev_queue_xmit() code
> > > path.
> > 
> > I don't agree with that part, I think this can be also beneficiary for
> > packets without direct xmit, as in PF_PACKET we don't have a notion of
> > "flow" but just raw packets instead, and can keep the mapping local
> > depending on the current CPU as we do queue setting elsewhere in the
> > stack just as well.
> 
> It seems to me that the newly added xmit in packet_sock is
> dev_queue_xmit() by default, and in this default case, dev_queue_xmit()
> would call netdev_pick_tx(), which would set the skb queue_mapping again
> to override the value based on the current CPU. 

Yes, I think you are right, that is also my experience with the code path.

> Or did I miss something here? 

A bit related; One thing I'm missing to understand, is why the
RAW/PF_PACKET sockets have a NULL in skb->sk when they reach
__netdev_pick_tx() ? (resulting in they cannot store/cache the queue in
sk_tx_queue_set)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

  reply	other threads:[~2013-12-12  7:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-12  6:10 [RFC PATCH net-next] packet: fix using smp_processor_id() in preemptible code Li Zhong
2013-12-11  9:55 ` Daniel Borkmann
2013-12-12 22:51   ` [PATCH v2 " Li Zhong
2013-12-12  7:19     ` Jesper Dangaard Brouer
2013-12-12  7:29     ` Daniel Borkmann
2013-12-12 21:15     ` David Miller
2013-12-13  1:49       ` Li Zhong
2013-12-12 23:55   ` [RFC PATCH " Li Zhong
2013-12-12  7:18     ` Jesper Dangaard Brouer [this message]
2013-12-13  6:41       ` Li Zhong
2013-12-13  7:40         ` Li Zhong
2013-12-11 21:49 ` David Miller
2013-12-12 22:54   ` Li Zhong

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=20131212081838.2d2eafb9@redhat.com \
    --to=jbrouer@redhat.com \
    --cc=brouer@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dborkman@redhat.com \
    --cc=linux-next@vger.kernel.org \
    --cc=zhong@linux.vnet.ibm.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.