All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: "Bjørn Mork" <bjorn@mork.no>
Cc: Kristian Evensen <kristian.evensen@gmail.com>,
	Network Development <netdev@vger.kernel.org>,
	subashab@codeaurora.org
Subject: Re: [PATCH net] qmi_wwan: Clone the skb when in pass-through mode
Date: Tue, 15 Jun 2021 12:26:04 -0700	[thread overview]
Message-ID: <20210615122604.1d68b37c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> (raw)
In-Reply-To: <871r93w8l9.fsf@miraculix.mork.no>

On Tue, 15 Jun 2021 15:39:14 +0200 Bjørn Mork wrote:
> >> I think this would be a really nice solution. The same (at least
> >> FLAG_MULTI_PACKET + usbnet_skb_return) could be applied to pass
> >> through as well, giving us consistent handling of aggregated packets.
> >> While we might not save a huge number of lines, I believe the
> >> resulting code will be easier to understand.  
> >
> > Apologies for the noise. When I check the code again, I see that as
> > long as FLAG_MULTI_PACKET is set, then we end up with usbnet freeing
> > the skb (we will always jump to done in rx_process()). So for the
> > pass-through case, I believe your initial suggestion of having
> > rx_fixup return 1 is the way to go.  
> 
> Yes, if we are to use FLAG_MULTI_PACKET then we must call
> usbnet_skb_return() for all the non-muxed cases.  There is no clean way
> to enable FLAG_MULTI_PACKET on-demand.

Tricky piece of code. Perhaps we could add another return code 
to the rx_fixup call? Seems that we expect 0 or 1 today, maybe we 
can make 2 mean "data was copied out", and use that for the qmimux 
case?

> I am fine with either solution.  Whatever Jakub wants :-)

Well, turns out I was looking at the wrong netif_rx() so take
what I say with a grain of salt ;)

  reply	other threads:[~2021-06-15 19:26 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-14 14:18 [PATCH net] qmi_wwan: Clone the skb when in pass-through mode Kristian Evensen
2021-06-14 14:45 ` Bjørn Mork
2021-06-14 15:49   ` Kristian Evensen
2021-06-14 17:02     ` Kristian Evensen
2021-06-14 20:05   ` Jakub Kicinski
2021-06-15  6:24     ` Bjørn Mork
2021-06-15  9:03     ` Kristian Evensen
2021-06-15 10:04       ` Bjørn Mork
2021-06-15 10:51         ` Kristian Evensen
2021-06-15 11:04           ` Kristian Evensen
2021-06-15 13:39             ` Bjørn Mork
2021-06-15 19:26               ` Jakub Kicinski [this message]
2021-06-15 19:27                 ` Jakub Kicinski
2021-06-16 10:08                 ` Kristian Evensen

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=20210615122604.1d68b37c@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com \
    --to=kuba@kernel.org \
    --cc=bjorn@mork.no \
    --cc=kristian.evensen@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=subashab@codeaurora.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 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.