Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1292546 > unrolled thread

Re: [PATCH] net: add Qualcomm IPC router

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] net: add Qualcomm IPC router David Miller <davem@davemloft.net> - 2015-12-15 22:10 +0100
    Re: [PATCH] net: add Qualcomm IPC router Dan Williams <dcbw@redhat.com> - 2015-12-15 22:20 +0100
    Re: [PATCH] net: add Qualcomm IPC router Dan Williams <dcbw@redhat.com> - 2015-12-15 22:20 +0100
    Re: [PATCH] net: add Qualcomm IPC router Courtney Cavin <courtney.cavin@sonymobile.com> - 2015-12-17 01:10 +0100
      Re: [PATCH] net: add Qualcomm IPC router David Miller <davem@davemloft.net> - 2015-12-17 01:20 +0100

#1292546 — Re: [PATCH] net: add Qualcomm IPC router

From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
Date: Fri, 11 Dec 2015 12:41:59 -0800

> +static unsigned int qrtr_local_nid = 1;
> +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> +MODULE_PARM_DESC(idVendor, "Local Node Identifier");

Module parameters suck.

Allow the user to choose this dynamically.  You have roughtly two choices.

1) Subvert the 'protocol' field passed to ->create() and use that, it is
   being ignored otherwise.

2) Put it into the socket address for bind().
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1292547

On Tue, 2015-12-15 at 15:13 -0600, Dan Williams wrote:
> On Tue, 2015-12-15 at 16:01 -0500, David Miller wrote:
> > From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> > Date: Fri, 11 Dec 2015 12:41:59 -0800
> > 
> > > +static unsigned int qrtr_local_nid = 1;
> > > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");
> 
> Also s/idVendor/node_id?

Well, not that it matters if the module parameter gets removed...

Dan

> 
> Dan
> 
> > Module parameters suck.
> > 
> > Allow the user to choose this dynamically.  You have roughtly two
> > choices.
> > 
> > 1) Subvert the 'protocol' field passed to ->create() and use that,
> > it
> > is
> >    being ignored otherwise.
> > 
> > 2) Put it into the socket address for bind().
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev"
> > in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1292548

On Tue, 2015-12-15 at 16:01 -0500, David Miller wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Date: Fri, 11 Dec 2015 12:41:59 -0800
> 
> > +static unsigned int qrtr_local_nid = 1;
> > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");

Also s/idVendor/node_id?

Dan

> Module parameters suck.
> 
> Allow the user to choose this dynamically.  You have roughtly two
> choices.
> 
> 1) Subvert the 'protocol' field passed to ->create() and use that, it
> is
>    being ignored otherwise.
> 
> 2) Put it into the socket address for bind().
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1293413

On Tue, Dec 15, 2015 at 10:01:14PM +0100, David Miller wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Date: Fri, 11 Dec 2015 12:41:59 -0800
> 
> > +static unsigned int qrtr_local_nid = 1;
> > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");
> 
> Module parameters suck.
> 
> Allow the user to choose this dynamically.  You have roughtly two choices.
> 
> 1) Subvert the 'protocol' field passed to ->create() and use that, it is
>    being ignored otherwise.
> 
> 2) Put it into the socket address for bind().

So each socket can have its own node id?  That doesn't seem right.

The way these node ids are assigned is by a system designer (in this
case Qualcomm).  The ARM, Linux CPU is always node 1, the audio DSP is
always node 5, etc.  Anyone with the knowhow could reassign these
numbers, but there's no reason to have them be dynamic during runtime.
Additionally, allowing dynamic assignment would require code to prevent
id duplication for known remote nodes, as well as to deal with cases in
which remote node discovery happens after local sockets have acquired
that node's id.

Maybe the first socket created needs CAP_NET_ADMIN, and uses the
'protocol' field to set the node id?  Ugh. Gross.

We could hardcode the value in kconfig, but that seems like a worse
solution than a module parameter.

I'm open to further suggestions.

-Courtney
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1293416

From: Courtney Cavin <courtney.cavin@sonymobile.com>
Date: Wed, 16 Dec 2015 16:01:41 -0800

> We could hardcode the value in kconfig, but that seems like a worse
> solution than a module parameter.
> 
> I'm open to further suggestions.

No module parameters, configure it via netlink or similar at run
time.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web