netdev - Re: [PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm

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:	Tue, 08 Mar 2016 21:18:15 +0100
From:	Bjørn Mork <bjorn@...k.no>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	David Miller <davem@...emloft.net>,
	Andrey Konovalov <andreyknvl@...il.com>,
	Oliver Neukum <oneukum@...e.com>,
	Dmitry Vyukov <dvyukov@...gle.com>,
	Alexander Potapenko <glider@...gle.com>,
	Kostya Serebryany <kcc@...gle.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	USB list <linux-usb@...r.kernel.org>,
	Network Development <netdev@...r.kernel.org>,
	Ben Hutchings <ben@...adent.org.uk>
Subject: Re: [PATCH] cdc_ncm: do not call usbnet_link_change from cdc_ncm_bind

Linus Torvalds <torvalds@...ux-foundation.org> writes:

> So looking at this, I wonder...
>
> Why is that FLAG_LINK_INTR thing not just always used?
>
> The _only_ thing that FLAG_LINK_INTR does is to cause
>
>         usbnet_link_change(dev, 0, 0);
>
> to be called after network device attach. That doesn't seem to be controversial.

Not all usbnet drivers support carrier detection, which is required to
ever bring the link up again.

> Looking at some examples, we have ax88179_178a.c that doesn't set the
> flag, but instead does that usbnet_link_change() call at the end of
> ax88179_bind().
>
> There are a few drivers that seem to never call that
> usbnet_link_change() at all, and don't have that FLAG_LINK_INTR flag.
> Would they break?

Yes.  Drivers without carrier detection will be "down" forever.

> Why is it called "FLAG_LINK_INTR" anyway? There doesn't seem to be
> anything "INTR" about it.

Beats me.  I can only say that I always find naming difficult...
We could ask Ben, who introduced it in:


commit 37e8273cd30592d3a82bcb70cbb1bdc4eaeb6b71
Author: Ben Hutchings <ben@...adent.org.uk>
Date:   Wed Nov 4 15:29:52 2009 +0000

    usbnet: Set link down initially for drivers that update link state
    
    Some usbnet drivers update link state while others do not due to
    hardware limitations.  Add a flag to distinguish those that do, and
    set the link down initially for their devices.
    
    This is intended to fix this bug: http://bugs.debian.org/444043
    
    Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
    Signed-off-by: David S. Miller <davem@...emloft.net>



But I guess it doesn't really matter.



Bjørn