Re: [PATCH net-next 2/3] net: qualcomm: rmnet: add tx packets aggregation

From: Alexander Lobakin <alexandr.lobakin@intel.com>
To: Daniele Palmas <dnlplm@gmail.com>
Cc: "Alexander Lobakin" <alexandr.lobakin@intel.com>,
	"David Miller" <davem@davemloft.net>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Eric Dumazet" <edumazet@google.com>,
	"Subash Abhinov Kasiviswanathan" <quic_subashab@quicinc.com>,
	"Sean Tranchetti" <quic_stranche@quicinc.com>,
	"Jonathan Corbet" <corbet@lwn.net>, "Bjørn Mork" <bjorn@mork.no>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net-next 2/3] net: qualcomm: rmnet: add tx packets aggregation
Date: Wed, 16 Nov 2022 17:20:16 +0100	[thread overview]
Message-ID: <20221116162016.3392565-1-alexandr.lobakin@intel.com> (raw)
In-Reply-To: <CAGRyCJHmNgzVVnGunUh7wwKxYA7GzSvfgqPDAxL+-NcO2P+1wg@mail.gmail.com>

From: Daniele Palmas <dnlplm@gmail.com>
Date: Wed, 16 Nov 2022 16:19:48 +0100

> Hello Alexander,
> 
> Il giorno gio 10 nov 2022 alle ore 18:35 Alexander Lobakin
> <alexandr.lobakin@intel.com> ha scritto:
> >
> > Do I get the whole logics correctly, you allocate a new big skb and
> > just copy several frames into it, then send as one chunk once its
> > size reaches the threshold? Plus linearize every skb to be able to
> > do that... That's too much of overhead I'd say, just handle S/G and
> > fraglists and make long trains of frags from them without copying
> > anything?
> 
> sorry for my question, for sure I'm lacking knowledge about this, but
> I'm trying to understand how I can move forward.
> 
> Suppose I'm able to build the aggregated block as a train of
> fragments, then I have to send it to the underlying netdevice that, in
> my scenario, is created by the qmi_wwan driver: I could be wrong, but
> my understanding is that it does not support fragments.
> 
> And, as far as I know, there's only another driver in mainline used
> with rmnet (mhi_net) and that one also does not seem to support them
> either.
> 
> Does this mean that once I have the aggregated block through fragments
> it should be converted to a single linear skb before sending?

Ah okay, I've missed the fact it's only an intermediate layer and
there's some real device behind it.
If you make an skb with fragments and queue it up to a netdev which
doesn't advertise %NETIF_F_SG, networking core will take care of
this. It will then form a set of regular skbs and queue it for
sending instead. Sure, there'll be some memcopies, but I can't say
this implementation is better until some stats provided.

And BTW, as Gal indirectly mentioned, those perf problems belong to
the underlying device, e.g. qmi_wwan and so on, rmnet shouldn't do
anything here. So you could try implement aggregation there or
whatever you'd like to pick. I'd try to read some specs first and
see whether qmi_wwan HW is capable of S/G or whether some driver
improvements for Tx could be done there.

> 
> Thanks,
> Daniele

Thanks,
Olek