mbox series

[v2,0/5] net: qrtr: Add distant node support

Message ID 1604684010-24090-1-git-send-email-loic.poulain@linaro.org
Headers show
Series net: qrtr: Add distant node support | expand

Message

Loic Poulain Nov. 6, 2020, 5:33 p.m. UTC
QRTR protocol allows a node to communicate with an other non-immediate 
node via an intermdediate immediate node acting as a 'bridge':

node-0 <=> node-1 <=> node-2

This is currently not supported in this upstream version and this
series aim to fix that.

This series is V2 because changes 1, 2 and 3 have already been submitted
separately on LKML.

Changes in v2:
- Add reviewed-by tags from Bjorn and Mani
- Fixing checkpatch issue reported by Jakub

Loic Poulain (5):
  net: qrtr: Fix port ID for control messages
  net: qrtr: Allow forwarded services
  net: qrtr: Allow non-immediate node routing
  net: qrtr: Add GFP flags parameter to qrtr_alloc_ctrl_packet
  net: qrtr: Release distant nodes along the bridge node

 net/qrtr/ns.c   |  8 --------
 net/qrtr/qrtr.c | 51 ++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 21 deletions(-)

Comments

Jakub Kicinski Nov. 8, 2020, 12:26 a.m. UTC | #1
On Fri,  6 Nov 2020 18:33:25 +0100 Loic Poulain wrote:
> QRTR protocol allows a node to communicate with an other non-immediate 
> node via an intermdediate immediate node acting as a 'bridge':
> 
> node-0 <=> node-1 <=> node-2
> 
> This is currently not supported in this upstream version and this
> series aim to fix that.
> 
> This series is V2 because changes 1, 2 and 3 have already been submitted
> separately on LKML.

Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
Is that correct?

If so first one needs to go to net and then onto 5.10, and the rest 
to net-next for 5.11.
Loic Poulain Nov. 9, 2020, 8:49 a.m. UTC | #2
Hi Jakub,

On Sun, 8 Nov 2020 at 01:26, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri,  6 Nov 2020 18:33:25 +0100 Loic Poulain wrote:
> > QRTR protocol allows a node to communicate with an other non-immediate
> > node via an intermdediate immediate node acting as a 'bridge':
> >
> > node-0 <=> node-1 <=> node-2
> >
> > This is currently not supported in this upstream version and this
> > series aim to fix that.
> >
> > This series is V2 because changes 1, 2 and 3 have already been submitted
> > separately on LKML.
>
> Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
> Is that correct?

That's correct, though strictly speaking 2-5 are also bug fix since remote node
communication is supposed to be supported in QRTR to be compatible with
other implementations (downstream or private implementations).

> If so first one needs to go to net and then onto 5.10, and the rest
> to net-next for 5.11.

I'm can split that into two series so that you can dispatch them at
your convenience.

Regards,
Loic
Jakub Kicinski Nov. 9, 2020, 6:39 p.m. UTC | #3
On Mon, 9 Nov 2020 09:49:24 +0100 Loic Poulain wrote:
> > Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
> > Is that correct?  
> 
> That's correct, though strictly speaking 2-5 are also bug fix since remote node
> communication is supposed to be supported in QRTR to be compatible with
> other implementations (downstream or private implementations).

Is there a spec we can quote to support that, or is QRTR purely 
a vendor interface?

What's the end user issue that we're solving? After firmware upgrade
things stop working? Things don't work on HW platforms on which this
was not tested? Don't work on new HW platforms?
Loic Poulain Nov. 10, 2020, 9:03 a.m. UTC | #4
Hi Jakub,

On Mon, 9 Nov 2020 at 19:39, Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 9 Nov 2020 09:49:24 +0100 Loic Poulain wrote:
> > > Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
> > > Is that correct?
> >
> > That's correct, though strictly speaking 2-5 are also bug fix since remote node
> > communication is supposed to be supported in QRTR to be compatible with
> > other implementations (downstream or private implementations).
>
> Is there a spec we can quote to support that, or is QRTR purely
> a vendor interface?

There is no public spec AFAIK, this is a vendor interface.

> What's the end user issue that we're solving? After firmware upgrade
> things stop working? Things don't work on HW platforms on which this
> was not tested? Don't work on new HW platforms?

QRTR is usually something used in SoC context as communication
protocol for accessing the differents IPs (modem, WiFi, DSP, etc)
around the CPU. In that case, these components (nodes), identified
with a 'node ID', are directly reachable by the CPU (QRTR over shared
memory). This case is not impacted by the series, all nodes beeing CPU
immediate neighbours.

But today QRTR is no more a ARCH_QCOM thing only, It is also exposed
as communication channel for QCOM based wireless modules (e.g. SDX55
modem), over PCIe/MHI. In that case, the host is only connected to the
Modem CPU QRTR endpoint that in turn gives access to other embedded
Modem endpoints, acting as a gateway/bridge for accessing
non-immediate nodes from the host. currently, this case is not working
and the series fix it.

However, AFAIK, the only device would request this support is the
SDX55 PCIe module, that just landed in mhi-next. So I assume it's fine
if the related part of the series targets net-next.

Regards,
Loic
Jakub Kicinski Nov. 10, 2020, 5:44 p.m. UTC | #5
On Tue, 10 Nov 2020 10:03:29 +0100 Loic Poulain wrote:
> On Mon, 9 Nov 2020 at 19:39, Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Mon, 9 Nov 2020 09:49:24 +0100 Loic Poulain wrote:  
> > > > Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
> > > > Is that correct?  
> > >
> > > That's correct, though strictly speaking 2-5 are also bug fix since remote node
> > > communication is supposed to be supported in QRTR to be compatible with
> > > other implementations (downstream or private implementations).  
> >
> > Is there a spec we can quote to support that, or is QRTR purely
> > a vendor interface?  
> 
> There is no public spec AFAIK, this is a vendor interface.
> 
> > What's the end user issue that we're solving? After firmware upgrade
> > things stop working? Things don't work on HW platforms on which this
> > was not tested? Don't work on new HW platforms?  
> 
> QRTR is usually something used in SoC context as communication
> protocol for accessing the differents IPs (modem, WiFi, DSP, etc)
> around the CPU. In that case, these components (nodes), identified
> with a 'node ID', are directly reachable by the CPU (QRTR over shared
> memory). This case is not impacted by the series, all nodes beeing CPU
> immediate neighbours.
> 
> But today QRTR is no more a ARCH_QCOM thing only, It is also exposed
> as communication channel for QCOM based wireless modules (e.g. SDX55
> modem), over PCIe/MHI. In that case, the host is only connected to the
> Modem CPU QRTR endpoint that in turn gives access to other embedded
> Modem endpoints, acting as a gateway/bridge for accessing
> non-immediate nodes from the host. currently, this case is not working
> and the series fix it.
> 
> However, AFAIK, the only device would request this support is the
> SDX55 PCIe module, that just landed in mhi-next. So I assume it's fine
> if the related part of the series targets net-next.

Thanks! Sounds like net-next will work just fine, but won't you need
these changes in mhi-next, then? In which case you should send a pull
request based on Linus' tree so that both Mani and netdev can pull it
in.

Mani, WDYT?
Jakub Kicinski Nov. 11, 2020, 11:31 p.m. UTC | #6
On Tue, 10 Nov 2020 09:44:27 -0800 Jakub Kicinski wrote:
> Thanks! Sounds like net-next will work just fine, but won't you need
> these changes in mhi-next, then? In which case you should send a pull
> request based on Linus' tree so that both Mani and netdev can pull it
> in.
> 
> Mani, WDYT?

Applied to net-next.
Manivannan Sadhasivam Nov. 12, 2020, 3:14 a.m. UTC | #7
On Tue, Nov 10, 2020 at 09:44:27AM -0800, Jakub Kicinski wrote:
> On Tue, 10 Nov 2020 10:03:29 +0100 Loic Poulain wrote:
> > On Mon, 9 Nov 2020 at 19:39, Jakub Kicinski <kuba@kernel.org> wrote:
> > >
> > > On Mon, 9 Nov 2020 09:49:24 +0100 Loic Poulain wrote:  
> > > > > Looks like patch 1 is a bug fix and patches 2-5 add a new feature.
> > > > > Is that correct?  
> > > >
> > > > That's correct, though strictly speaking 2-5 are also bug fix since remote node
> > > > communication is supposed to be supported in QRTR to be compatible with
> > > > other implementations (downstream or private implementations).  
> > >
> > > Is there a spec we can quote to support that, or is QRTR purely
> > > a vendor interface?  
> > 
> > There is no public spec AFAIK, this is a vendor interface.
> > 
> > > What's the end user issue that we're solving? After firmware upgrade
> > > things stop working? Things don't work on HW platforms on which this
> > > was not tested? Don't work on new HW platforms?  
> > 
> > QRTR is usually something used in SoC context as communication
> > protocol for accessing the differents IPs (modem, WiFi, DSP, etc)
> > around the CPU. In that case, these components (nodes), identified
> > with a 'node ID', are directly reachable by the CPU (QRTR over shared
> > memory). This case is not impacted by the series, all nodes beeing CPU
> > immediate neighbours.
> > 
> > But today QRTR is no more a ARCH_QCOM thing only, It is also exposed
> > as communication channel for QCOM based wireless modules (e.g. SDX55
> > modem), over PCIe/MHI. In that case, the host is only connected to the
> > Modem CPU QRTR endpoint that in turn gives access to other embedded
> > Modem endpoints, acting as a gateway/bridge for accessing
> > non-immediate nodes from the host. currently, this case is not working
> > and the series fix it.
> > 
> > However, AFAIK, the only device would request this support is the
> > SDX55 PCIe module, that just landed in mhi-next. So I assume it's fine
> > if the related part of the series targets net-next.
> 
> Thanks! Sounds like net-next will work just fine, but won't you need
> these changes in mhi-next, then? In which case you should send a pull
> request based on Linus' tree so that both Mani and netdev can pull it
> in.
> 
> Mani, WDYT?

Sorry, missed this. mhi-next doesn't need these changes and since you've
applied to net-next, everything is fine!

I still need to apply the MHI patch which got applied to net-next and
provide an immutable branch to Kalle for another set of MHI patches...

Thanks,
Mani