netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: dada1@cosmosbay.com
Cc: netdev@vger.kernel.org, jussi.kivilinna@mbnet.fi
Subject: Re: [PATCH] sch_teql: should not dereference skb after ndo_start_xmit
Date: Sun, 02 Aug 2009 18:59:41 -0700 (PDT)	[thread overview]
Message-ID: <20090802.185941.120840905.davem@davemloft.net> (raw)
In-Reply-To: <4A120C8B.80108@cosmosbay.com>

From: Eric Dumazet <dada1@cosmosbay.com>
Date: Tue, 19 May 2009 03:34:03 +0200

> Looking again at teql_master_xmit(), I wonder if there is another
> problem in it.
> 
>     int subq = skb_get_queue_mapping(skb);
> 
> But as a matter of fact, following code assumes subq is 0

I looked into this again, and damn this is tough to deal with.
The code works as-is, since teql devices have only 1 queue we
can assume queue 0 and we only end up using one of the slave
devices queues too.

I tried to export dev_pick_tx() (renaming it to netdev_pick_tx()
to avoid global namespace pollution) but then I realized that
we can't just whack the subq here.

If this gets punted back to the caller and we don't actually
send out the packet, we can't leave the new skb->queue_index
in there as teql's multiqueue parameters are what will be
checked and used against this SKB again.

I suppose we could restore the old queue index value when we
exhaust the slaves and can't transmit, but is getting messy for
sure.

Since the code works properly, and this is merely a performance
issue, I'm deferring this again.

      parent reply	other threads:[~2009-08-03  1:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-18 10:31 [PATCH] sch_teql: should not dereference skb after ndo_start_xmit Eric Dumazet
2009-05-18 22:12 ` David Miller
2009-05-19  1:34   ` Eric Dumazet
2009-05-19  2:34     ` David Miller
2009-05-19  5:43       ` [PATCH] sch_teql: Use net_device internal stats Eric Dumazet
2009-05-19 22:16         ` David Miller
2009-08-03  1:59     ` David Miller [this message]

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=20090802.185941.120840905.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=dada1@cosmosbay.com \
    --cc=jussi.kivilinna@mbnet.fi \
    --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;
as well as URLs for NNTP newsgroup(s).