netdev - Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is disabled

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:	Thu, 7 Mar 2013 19:10:33 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Veaceslav Falico <vfalico@...hat.com>
CC:	David Miller <davem@...emloft.net>, <netdev@...r.kernel.org>,
	<wfp5p@...ginia.edu>, <jasowang@...hat.com>,
	<junchangwang@...il.com>, <greearb@...delatech.com>,
	<ivecera@...hat.com>
Subject: Re: [PATCH] 8139too: send NETDEV_CHANGE manually when autoneg is
 disabled

On Thu, 2013-03-07 at 19:38 +0100, Veaceslav Falico wrote:
> On Thu, Mar 07, 2013 at 04:54:21PM +0000, Ben Hutchings wrote:
> >On Thu, 2013-03-07 at 17:35 +0100, Veaceslav Falico wrote:
> >> On Thu, Mar 07, 2013 at 03:52:35PM +0000, Ben Hutchings wrote:
> >> >On Thu, 2013-03-07 at 11:27 +0100, Veaceslav Falico wrote:
[...]
> >> >> Silently changing speed can break things a bit - bonding relies on
> >> >> interface speeds for 802.3ad/alb/tlb/active-backup iirc, bridge relies on
> >> >> stp port cost etc. and they all get it via NETDEV_ notifications. So
> >> >> without them, they would end up with outdated data, per example (eth2 being
> >> >> 8139too):
> >> >[...]
> >> >
> >> >Yes, I get it.  But on real hardware, changing speed/duplex is always
> >> >going to break the link if it's up.  The link change notification should
> >> >result in kernel and userland notifications via linkwatch_do_dev().
> >> >
> >> >(If you're testing against QEMU rather than real hardware, there could
> >> >be a bug in the emulation of link interrupts.)
> >>
> >> It's real hardware:
> >>
> >> [    4.339413] 8139cp 0000:10:09.0: This (id 10ec:8139 rev 10) is not an 8139C+ compatible chip, use 8139too
> >> [    4.348210] 8139too: 8139too Fast Ethernet driver 0.9.28
> >> [    4.350017] 8139too 0000:10:09.0 eth2: RealTek RTL8139 at 0xffffc90004574100, 00:14:d1:1f:b9:49, IRQ 21
> >[...]
> >
> >OK.  But it's generally not enough to send a 'something changed'
> >notification; the driver actually has to update the link state.  MAybe
> >in your test you reconfigure the other end of the link too, or it's
> >smart enough to do parallel detect.  But in general, changing the link
> >speed is going to change the link state, and the TX scheduler needs to
> >enabled or disabled accordingly.
> 
> Sorry, I think I didn't explain it clearly. The driver *does* notify
> about link changes even when it has autonegotiation disabled.
> 
> The only thing that doesn't work is the notification when someone changes
> it via ethtool. In the same moment.

The link *should* go down (assuming it was up before) because you likely
have a speed mismatch and won't be able to pass traffic.  I don't know
whether this is possible for the hardware to detect immediately; maybe a
PHY reset will ensure that it is detected.

[...]
> Other drivers, after setting autoneg off, usually ifdown/ifup the interface
> manually, and thus everything gets updated. That is another way to fix this
> driver - to issue ->ndo_stop(); ->ndo_open(); (as netxen_nic/qlcnic/etc
> do), but I think that just issuing a notification that we've changed the
> state, without flapping, is better.

The link *should* flap, though doing a full stop/start is likely
unnecessary.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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