netdev - Re: [PATCH net-next V3 02/11] net/mlx5_core: Add EQ renaming mechanism

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 14 May 2015 18:19:48 +0300
From:	Or Gerlitz <gerlitz.or@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev <netdev@...r.kernel.org>,
	Or Gerlitz <ogerlitz@...lanox.com>,
	Tal Alon <talal@...lanox.com>,
	Achiad Shochat <achiad@...lanox.com>,
	Saeed Mahameed <saeedm@...lanox.com>
Subject: Re: [PATCH net-next V3 02/11] net/mlx5_core: Add EQ renaming mechanism

On Tue, May 12, 2015 at 2:55 PM, Amir Vadai <amirv@...lanox.com> wrote:
> On Mon, May 11, 2015 at 8:40 PM, David Miller <davem@...emloft.net> wrote:
>> From: Amir Vadai <amirv@...lanox.com>
>> Date: Sun, 10 May 2015 12:38:57 +0300
>>
>>> +int mlx5_rename_eq(struct mlx5_core_dev *dev, int eq_ix, char *name)
>>> +{
>>> +     struct mlx5_priv *priv = &dev->priv;
>>> +     struct mlx5_eq_table *table = &priv->eq_table;
>>> +     struct mlx5_eq *eq;
>>> +     int err = -ENOENT;
>>> +
>>> +     spin_lock(&table->lock);
>>> +     list_for_each_entry(eq, &table->comp_eqs_list, list) {
>>> +             if (eq->index == eq_ix) {
>>> +                     snprintf(priv->irq_info[eq_ix].name, MLX5_MAX_IRQ_NAME,
>>> +                              "%s-%d", name, eq_ix);
>>> +                     err = 0;
>>> +                     break;
>>> +             }
>>> +     }
>>> +     spin_unlock(&table->lock);
>>> +
>>> +     return err;
>>> +}
>>
>> You have to be very careful with this.
>>
>> If you change these names after the request_irq() call(s), the new name string
>> will not propagate to /proc/interrupts output etc.
>>
>> Looking at your later patches, this seems to be invoked very late in
>> mlx5e_open_locked(), so I am concerned.
> It is true. We call request_irq() when driver is loaded (and we don't
> know yet if the ports types are Infiniband or Ethernet). Only later
> on, we rename the name when interface is up and we know its name.
>
> But, empirically /proc/interrupts is propagated with the new name
> (according to latest net-next). Also, in the code, I don't see why
> shouldn't it be updated. When calling request_irq(), name is not
> copied, but irq_desc->action->name is pointing to it. This same
> pointer is being used by show_interrupts() when /proc/interrupts is
> queried.
> So don't see why when modifying the string pointed by it, it wouldn't
> shown on /proc/interrupts.

Dave, does this makes sense to you?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html