Re: [PATCH 1/9] Ignore rild modem devices.

[Thomas Haller]

On Wed, 2016-06-08 at 16:20 -0400, Tony Espy wrote:
> From: Mathieu Trudel-Lapierre 
> 
> Gbp-Pq: Name Ignore-rild-modem-devices.patch
> ---
>  src/nm-manager.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/nm-manager.c b/src/nm-manager.c
> index 10aa3d7..c2ed1da 100644
> --- a/src/nm-manager.c
> +++ b/src/nm-manager.c
> @@ -2089,6 +2089,14 @@ platform_link_added (NMManager *self,
>   gboolean ignore = FALSE;
>   gs_free_error GError *error = NULL;
>  
> + /* Ignore rild modem devices, which will be handled
> by their modem parent */
> + if (g_strstr_len (plink->name, NM_STRLEN ("rmnet"),
> "rmnet") ||
> + g_strstr_len (plink->name, NM_STRLEN
> ("rev_rmnet"), "rev_rmnet") ||
> + g_strstr_len (plink->name, NM_STRLEN
> ("ccmni"), "ccmni")) {
> + _LOGW (LOGD_HW, "Ignoring rild modem device:
> %s", plink->name);
> + return;
> + }
> +
>   device = nm_device_factory_create_device (factory,
> plink->name, plink, NULL, , );
>   if (!device) {
>   if (!ignore) {


  g_strstr_len (plink->name, NM_STRLEN ("rmnet"), "rmnet")
is identical to
  g_str_has_prefix (plink->name, "rmnet")
or
  strncmp (plink->name, "rmnet", NM_STRLEN ("rmnet")) == 0
isn't it?


g_str_has_prefix() seems easier to understand in this case.



signature.asc
Description: This is a digitally signed message part
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Re: [PATCH 1/9] Ignore rild modem devices.

[Tony Espy]

On 06/08/2016 05:37 PM, Dan Williams wrote:

On Wed, 2016-06-08 at 16:20 -0400, Tony Espy wrote:

From: Mathieu Trudel-Lapierre 

Gbp-Pq: Name Ignore-rild-modem-devices.patch
---
  src/nm-manager.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/src/nm-manager.c b/src/nm-manager.c
index 10aa3d7..c2ed1da 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -2089,6 +2089,14 @@ platform_link_added (NMManager *self,
gboolean ignore = FALSE;
gs_free_error GError *error = NULL;

+   /* Ignore rild modem devices, which will be handled
by their modem parent */
+   if (g_strstr_len (plink->name, NM_STRLEN ("rmnet"),
"rmnet") ||
+   g_strstr_len (plink->name, NM_STRLEN
("rev_rmnet"), "rev_rmnet") ||
+   g_strstr_len (plink->name, NM_STRLEN
("ccmni"), "ccmni")) {
+   _LOGW (LOGD_HW, "Ignoring rild modem device:
%s", plink->name);
+   return;
+   }


I'd actually put this into the WWAN factory, see just below:


device = nm_device_factory_create_device (factory,
plink->name, plink, NULL, , );
if (!device) {
if (!ignore) {


where  is; the factory can indicate to the manager that it wants
certain devices ignored through that variable.  So you'd toss this into
src/devices/wwan/nm-wwan-factory.c::create_device() instead.


The one area of concern I have is whether or not these devices are 
reported as the proper type, otherwise they won't get passed to the wwan 
factory.


If this isn't a problem for us, then this seems like a reasonable 
approach... except for the fact that we've hard-coded these device 
matching expressions.  Over time this won't scale.


Any thoughts about how this could be made runtime configurable?  I'd 
love to get rid of one more source patch.  ;)


/tony

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

Re: [PATCH 1/9] Ignore rild modem devices.

[Dan Williams]

On Wed, 2016-06-08 at 16:20 -0400, Tony Espy wrote:
> From: Mathieu Trudel-Lapierre 
> 
> Gbp-Pq: Name Ignore-rild-modem-devices.patch
> ---
>  src/nm-manager.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/nm-manager.c b/src/nm-manager.c
> index 10aa3d7..c2ed1da 100644
> --- a/src/nm-manager.c
> +++ b/src/nm-manager.c
> @@ -2089,6 +2089,14 @@ platform_link_added (NMManager *self,
>   gboolean ignore = FALSE;
>   gs_free_error GError *error = NULL;
>  
> + /* Ignore rild modem devices, which will be handled
> by their modem parent */
> + if (g_strstr_len (plink->name, NM_STRLEN ("rmnet"),
> "rmnet") ||
> + g_strstr_len (plink->name, NM_STRLEN
> ("rev_rmnet"), "rev_rmnet") ||
> + g_strstr_len (plink->name, NM_STRLEN
> ("ccmni"), "ccmni")) {
> + _LOGW (LOGD_HW, "Ignoring rild modem device:
> %s", plink->name);
> + return;
> + }

I'd actually put this into the WWAN factory, see just below:

>   device = nm_device_factory_create_device (factory,
> plink->name, plink, NULL, , );
>   if (!device) {
>   if (!ignore) {

where  is; the factory can indicate to the manager that it wants
certain devices ignored through that variable.  So you'd toss this into
src/devices/wwan/nm-wwan-factory.c::create_device() instead.

Dan
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list