netdev - Re: [PATCH net-next V4 00/12] net/mlx5: ConnectX-4 100G Ethernet driver

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 27 May 2015 20:57:37 +0100
From:	Ben Hutchings <ben@...adent.org.uk>
To:	David Miller <davem@...emloft.net>
Cc:	gerlitz.or@...il.com, netdev@...r.kernel.org,
	ogerlitz@...lanox.com, talal@...lanox.com, achiad@...lanox.com,
	saeedm@...lanox.com, amirv@...lanox.com, edumazet@...gle.com,
	tom@...bertland.com, jiri@...nulli.us, bpoirier@...e.de,
	nhorman@...driver.com
Subject: Re: [PATCH net-next V4 00/12] net/mlx5: ConnectX-4 100G Ethernet
 driver

On Wed, 2015-05-27 at 13:36 -0400, David Miller wrote:
> From: Or Gerlitz <gerlitz.or@...il.com>
> Date: Mon, 25 May 2015 22:07:23 +0300
> 
> > On Wed, May 20, 2015 at 5:17 PM, Amir Vadai <amirv@...lanox.com> wrote:
> >> On Tue, May 19, 2015 at 7:41 PM, David Miller <davem@...emloft.net> wrote:
> >>> From: Amir Vadai <amirv@...lanox.com>
> >>> Date: Tue, 19 May 2015 12:25:12 +0300
> >>>
> >>>> On Sun, May 17, 2015 at 8:05 PM, David Miller <davem@...emloft.net> wrote:
> >>>>> From: Amir Vadai <amirv@...lanox.com>
> >>>>> Date: Sun, 17 May 2015 16:02:11 +0300
> >>>>>
> >>>>>> We didn't get a response yet regarding your comment about the irq
> >>>>>> renaming [3].
> >>>>>
> >>>>> Well then, please hold off on resubmissions of this series until you
> >>>>> do get a response and that issue is firmly resolved.
> >>>> Hi,
> >>>>
> >>>> I don't mean to push you, I only want to understand what is expected
> >>>> from me and what are the next steps:
> >>>> How will the issue be resolved? Do you plan to answer my question [1]
> >>>> from last week, and just too busy right now or something like that?
> >>>
> >>> I have not seen any response to me explaining why it's ok to change
> >>> the IRQ name strings in the context where this will occur.
> >>>
> >>> Once you explain that, we can make forward progress, but only at that
> >>> point.
> >>
> >> Hi Dave,
> >>
> >> Just to put us back on the same page, repeating a bit the previous chapters..
> >>
> >> You wrote [1] that if we change these names after the request_irq() call(s), the
> >> new name string will not propagate to /proc/interrupts output.
> >>
> >> So, indeed, request_irq() is called when the driver is loaded (and we
> >> don't know yet if the port types are Infiniband or Ethernet). Only later
> >> on, we rename the name when the Ethernet interface is up and we know
> >> its name.
> >>
> >> Fact is that the new name does propagate to /proc/interrupts.
> >>
> >> Also, looking in the code, I don't see a reason why shouldn't it
> >> be properly updated. When calling request_irq(), the name argument
> >> is not copied, but irq_desc->action->name points to it. This same pointer
> >> is being used by show_interrupts() when /proc/interrupts is shown.
> 
> The problem is, applications might cache the name of the interrupt source
> from /proc/interrupts or elsewhere as soon as the device is instantiated.
>
> You cannot change it after you've published it there.

How would an application tell the difference between an IRQ handler
being renamed, or being unregistered and re-registered under a different
name?  I'm fairly sure it can't tell.

> Therefore, it is mandatory that the name string is at the time you call
> request_irq().

It certainly ought to be something non-null and a useful identifier for
the device.  But the convention of including the net device name means
that either the interrupt handler should be registered only when the
device is up, or it should be renamed when the device is.

There are certainly some network drivers that handle interrupts while
the device is down, e.g. because they have a PHC.  Should those
interrupt handlers be named after the PCI device instead?

Ben.

-- 
Ben Hutchings
Reality is just a crutch for people who can't handle science fiction.

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)