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

From: Daniele Palmas <dnlplm@gmail.com>
To: Alexander Lobakin <alexandr.lobakin@intel.com>
Cc: "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: Sun, 20 Nov 2022 10:25:53 +0100	[thread overview]
Message-ID: <CAGRyCJHX9WMeHLBgh5jJj2mNJh3hqzAhHacVnLqP_CpoHQaTaw@mail.gmail.com> (raw)
In-Reply-To: <20221116162016.3392565-1-alexandr.lobakin@intel.com>

Il giorno mer 16 nov 2022 alle ore 17:21 Alexander Lobakin
<alexandr.lobakin@intel.com> ha scritto:
>
> 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.

Ok, so rmnet would only take care of qmap rx packets deaggregation and
qmi_wwan of the tx aggregation.

At a conceptual level, implementing tx aggregation in qmi_wwan for
passthrough mode could make sense, since the tx aggregation parameters
belong to the physical device and are shared among the virtual rmnet
netdevices, which can't have different aggr configurations if they
belong to the same physical device.

Bjørn, would this approach be ok for you?

Thanks,
Daniele

> 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